Re: [RFC] client-side cleanup

 On Tue, 29 Mar 2011 11:15:18 -0600, Alex Rousskov wrote:
> On 03/26/2011 03:49 AM, Amos Jeffries wrote:
>> On the cleanup branch I've been going through and trying to document
>> what the client-side classes do.
>>
>> As you probably know their names only have loose affiliation with
>> their
>> operation and there is a LOT of scope creep blurring the boundaries.
>>
>> Below are the steps I'm seriously considering making to trunk over
>> the
>> new few weeks in order to be able to clarify and document the scopes
>> properly.
>>
>> 1) I would like to rename ConnStateData to "ConnectionManager" since
>> its
>> core scope seems to be:
>> * owning the client FD
>> * owning the request transaction state object(s)
>> * passing data to the request parser
>> * managing read/write ops with the client-streams buffers
>> ie (ultimate Producer for request and Consumer for reply)
>
> I think the name should be specific to the client side unless we add
> a
> Clt namespace or some such: Clt::ConnMgr or CltConnMgr should work.
> You
> can spell those short names out more if you prefer, of course.
>
> And let's start documenting somewhere what we think those classes do
> now
> (and what we change them to do next). Is Wiki the best place for
> that?
>
>
>> 2) I would like to rename ClientSocketContext as ClientXaction since
>> its
>> core scope seems to be owning the processing state flags.
>
> There is definitely a client transaction hidden among all those
> client-side classes. I am not sure which class should be renamed. If
> you
> think ClientSocketContext is it, and nobody has better ideas, let's
> try
> that.
>
>
>> This would eventually become the master transaction object, storing
>> the
>> log history and master pointers to bits of data.
>
> That would be wrong, IMO. The Master Transaction class should not be
> tied to the client-side or any "side" transaction management. The
> master
> object must survive when the client disconnects, for example, because
> we
> may still need it for finishing adaptation, logging, etc.
>
> I would not be surprised if that MasterXact class will have virtually
> no
> "processing" code of its own, just transaction history, metadata
> management, and sides coordination.
>
>
>> Although there is a lot of change before it gets to be used
>> everywhere.
>> Starting with turning its doCallouts with Async steps.
>
> One of the first steps would be to move code that belongs to one
> class
> from all the random client*cc files it appears in into the file
> dedicated to that class. That may be more difficult than it sounds,
> but
> it may also help you identify class boundaries and roles better.
>
>
>> 3) deflating the parser levels and documenting the structure of
>> ownership for each parse step.
>> I've started with breaking HttpParser out of HttpMsg.*.
>>
>>
>> 4) figuring out if we actually need ClientHttpRequest after the
>> above.
>>
>>
>> Ideas? opinions? stuff I've overlooked?
>
> The three biggest problems with the client side that I constantly
> bump
> into is
>
> (a) it is not clear which class is responsible for what
> (b) sharing of client-side objects (everybody points to everybody)
> (c) ClientStream API (low level pointer and other's buffer
> manipulation)
>
> You are already working on improving (a), thank you. Anything you can
> do
> to help with (b) and (c) would be welcomed.
>
>
> Please do not do many things at once. If you are renaming and moving
> code then avoid any other changes -- it becomes impossible to review
> those correctly. If you are changing code in other ways, please keep
> your changes to the minimum so that we have a better chance of
> understanding the consequences while being surrounded by the client
> side
> mess.

 Aye, I learnt that lesson with comm-cleanup. The auth changes going in
 now are a good example of how I hope to go forward in all my projects.
 Baby steps of one change, skipping all those with larger side effects.
 Audit on everything which changes behaviour.

>
> Finally, I am very concerned with your plan to commit the above
> changes
> to trunk when 3.2 is not stable yet. If the above changes are not
> limited to simple renaming/moving, those of us who work on the
> remaining
> 3.2 stability projects would have to switch from trunk code to the
> 3.2
> branch. As you know from 3.0 and especially 3.1 experience, this will
> significantly reduce the amount of changes going back to trunk
> because
> we will be confined to our 3.2-based branches that will move away
> from
> trunk pretty fast (unlike today when we can work with trunk and trust
> you to carefully copy changes to the official 3.2 branch).

 I am *definitely* hoping to avoid anything beyond simple renaming and
 documentation.
 Though as you noted the SourceLayout shuffles would be good thing.

>
> IMO, the above changes to trunk should be postponed until we make
> v3.2
> stable, including performance regressions, as discussed earlier.

 Okay.

 Amos
Received on Wed Mar 30 2011 - 00:37:02 MDT