Re: [RFC] SourceLayout for client-side and server-side components

Okay. We seem to be in agreement about the parser/packer code. The rst I
guess we can wait a bit and see if it beccomes clearer once that code is
shuffled out.

On 16/03/2013 4:32 a.m., Alex Rousskov wrote:
> On 03/14/2013 09:55 PM, Amos Jeffries wrote:
>> On 14/03/2013 1:49 p.m., Alex Rousskov wrote:
>>> On 03/13/2013 05:10 AM, Amos Jeffries wrote:
>>>> 1) we retain the client-side/server-side naming and place the
>>>> components in a sub-directory off the main protocol directory.
>>>> src/http/ - HTTP protocol definitions
>>>> src/http/client - client-side HTTP receiving component
>>>> src/http/server - client-side HTTP receiving component
>
>>> this layout is upside down IMO: Protocol is less
>>> important than the "side" from Squid architecture point of view. Squid
>>> has only a few "sides". There can be ten or more protocols working on
>>> each side. And some protocols work only on one side.
>
>> This is my proposal #2, with s/receivers/CCC/ s/senders/SSS/
> Well, any N-level directory layout proposal can be made look like any
> other proposal by renaming directories. The key here is to define what
> those directories contain. As discussed, receivers are very different
> from CCC and senders are very different from SSS. To avoid confusion,
> please do not use receiver/sender terminology unless you actually do
> mean "receiver of a message" and "sender of a message" like HTTPbis does.
>
> Said that, I am happy to use "#2" for the CCC/SSS layout below:
>
>>> src/http - HTTP-specific classes used by both sides
>>> src/CCC - classes dealing with clients (all protocols)
>>> src/SSS - classes dealing with servers (all protocols)
> Perhaps it would be useful to clarify the above by saying that:
>
> a) There are other Squid sides not shown above (e.g. adaptation).
> We just use CCC and SSS as two primary examples.

This is a good example why splitting it by side at the upper level
rather than by protocol is _worse_. I'm after consistency here - if you
are looking for the parser from ftp-client code or http-client or
gopher-client or icap-client you should expect to be able to look in a
similar place.

> b) There are other protocols not shown above (e.g. FTP).
> We just use HTTP as a primary example.

Another example of why splitting by 'side' is bad. FTP protocol stuff
does not get used in adapted/, nor does gopher/ or icp/ or htcp/. But
icap/ does get used in both client-side/ and server-side/.

> c) Any HTTP code used by more than one side belongs to src/http/
> d) Any code used by more than one side does not belong to src/side/

Okay. Agreed on that.

When you look at it this way ICAP protocol parse/pack belongs under an
src/icap/ as much as src/http/ deserves its own. Because it is used by
adaptation at both REQMOD and RESPMOD "sides".

>
>> I saw some issues with the #2 layout, which made me switch to #1 as the
>> preferred:
>> * it requires the client-side parser logics which are _specific_ to one
>> protocol to be separated from that protocols definitions and shunted
>> down below CCC/ and likewise for the request packer routines to be
>> shunted out and under SSS/.
> I do not see such a requirement. Protocol parsing and packing code
> should live in protocol-specific directories. It does not deal with any
> specific Squid side. The parsing and packing code should be specific to
> the protocol, not side (but may have hooks for a side to optimize or
> tune packing/parsing). For example, the adaptation side of Squid needs
> to both parse and pack HTTP messages.

That is the code I'm talking about.

>
>> I would much rather have that type of code shunted to http/*.h and
>> http/*.cc files and have http/CCC/*.cc include http/*.h.
> And that is what we should do.
>
>
>> * We can exclude entire protocol components by omiting a whole library
>> X/*.la from linking instead of doing the exclusion in multiple places
>> separately for all the CCC/* files and SSS/* files. This in turn opens
>> us to the option of dynamically linked/loaded FTP support or CoAP
>> support or HTTPS support, etc. just by linking the required src/X/*
>> library as a whole.
> IMO, the primary difficulty of protocol exclusion lies in keeping the
> general (all-protocols) code that uses the protocol working after the
> protocol support has been disabled. We currently do not have enough APIs
> to support such exclusion and, frankly, I do not think we should make
> things more complex by introducing such APIs now. The CCC/SSS design
> allows for adding such APIs in the future.

Ack.

>
>> Note that all this applies to only the parser/packer code. We will still
>> need src/X/ and src/CCC/ and src/SSS/ directories whichever one we end
>> up shuffling the protocol-specific parser/packer code into.
> Packing and parsing code for protocol P goes into src/p/. I see no other
> alternative that will work. That is why I snipped most of the discussion
> about problems caused by that code being placed elsewhere.
>
>
> Thank you,
>
> Alex.
>
Received on Fri Mar 15 2013 - 22:36:49 MDT