Re: [PREVIEW] Dechunk incoming requests as needed and pipeline them to the server side.

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Sun, 05 Sep 2010 02:28:26 +1200

Alex Rousskov wrote:
> Dechunk incoming requests as needed and pipeline them to the server side.
>
> The server side will eventually either chunk the request or fail. That
> code is not ready yet and is not a part of this patch. This patch iis
> not enough because the dechunked requests end up being sent without
> chunking and without Content-Length. However, these client-side changes
> are ready and seem to be working. It may be easier to review them now,
> without the server-side code.
>
> Details are below.
>
>
> Removed clientIsRequestBodyValid() as unused. It was called with a
> content-length>0 precondition that made the function always return true.
>
> Removed old dechunking hack that was trying to buffering the entire
> request body, pretending that we are still reading the headers. Adjusted
> related code. More work may be needed to identify client-side code that
> assumes the request size is always known.
>
> Removed ConnStateData::bodySizeLeft() because we do not always know how
> much body is left to read -- chunked requests do not have known sizes
> until we read the last-chunk. Moreover, it was possibly used wrong
> because sometimes we want to know whether we want to comm_read more body
> bytes and sometimes we want to know whether we want to "produce" more
> body bytes (i.e., copy already read bytes into the BodyPipe buffer,
> which can get full).
>
> Added ConnStateData::mayNeedToReadMoreBody() to replace
> conn->bodySizeLeft() with something more usable and precise.
>
> Removed my wrong XXX related to closing after initiateClose.
>
> Removed my(?) XXX related to endless chunked requests. There is nothing
> special about them, I guess, as a non-chunked request can be virtually
> endless as well if it has a huge Content-Length value.
>
> Use commIsHalfClosed() instead of fd_table[fd].flags.socket_eof for
> consistency with other client-side code and to improve readability. I
> think these should return the same value in our context but I am not sure.
>
> Correctly handle identity encoding. TODO: double check that it is still
> in the HTTP standard.
>
> Fixed HttpStateData::doneSendingRequestBody to call its parent. I am not
> sure it helps with correctly processing transactions, but the parent
> method was designed to be called, and calling it make the transaction
> state more clear.
>
>
> Thank you,
>
> Alex.
>

If your comment is correct and connKeepReadingIncompleteRequest() only
refer to the headers. Could the function name be updated too say that?

In connCancelIncompleteRequests the debugs look useless for level 1.
There is no info about which requests they refer. If you wish to retain
them at that level please use DBG_IMPORTANT, reference something to
track the request and add a WARNING or NOTICE etc tag to highlight why
they are at that level.

Looks good so far.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.7
   Beta testers wanted for 3.2.0.1
Received on Sat Sep 04 2010 - 14:28:40 MDT

This archive was generated by hypermail 2.2.0 : Mon Sep 06 2010 - 12:00:04 MDT