Re: [RFC] client-side cleanup

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.

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).

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

Thank you,

Alex.
Received on Tue Mar 29 2011 - 17:15:36 MDT