Re: [Libevent-users] http server and infinite streams

[Scott Lamb <slamb@xxxxxxxxx>]

On Fri, May 6, 2011 at 10:07 PM, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:
> On Sat, Apr 30, 2011 at 9:55 PM, Cliff Frey <cliff@xxxxxxxxxx> wrote:
>> I added tests, handled the infinite-receive case as well as the
>> infinite-send case, and slightly renamed a few things.
>>
>> https://github.com/clifffrey/Libevent/tree/http-transfer-throttling
>>
>> I believe that the changes there are enough to prevent out-of-memory
>> conditions when sending or receiving huge HTTP requests.
>>
>> Part of me wonders if this is the wrong way to go with the fixes, and
>> if it would somehow be better/natural to make the HTTP layer act a
>> little bit more like a filter, and expose a struct bufferevent
>> interface to the incoming/outgoing data stream.  However, that seems
>> like it would end up being a bigger rewrite, and this is a concrete
>> fix to a specific problem, so this change still feels worthwhile to
>> me.
>
> Hi!  I was going to try to think more and post on Monday with more
> cogent review, but I hear that a review before the weekend gets
> underway would be of more use to you, so I'm going to substitute speed
> for cogency: I hope that the result is still useful.
>
> So I like the idea of getting read-by-read notification of data as it
> arrives.  I'm not thrilled with the idea of changing default behavior
> wrt what counts as a read, though.  Either a flag or another kind of
> callback would make me much more comfortable about the change here.

IMHO, this is fine; the behavior Cliff is changing wasn't documented
and wasn't a reasonable thing to depend on. The boundaries of HTTP
chunks better not have any meaning to the application because they
could be changed by a proxy server along the way.

Furthermore, I think it's important that the "chunked cb" be called as
data is read even if the server didn't use chunked encoding. Something
like it is obviously necessary to avoid running of memory on large
requests, and given the above I don't see any reason to define a
different interface.

>
> I think that the evhttp_send_reply_chunk_with_cb behavior might be a
> little screwy.  Suppose that you send one chunk with a cb, then send
> another chunk with cb == NULL.  The second time you call
> evhttp_send_reply_chunk_with_cb, it won't clear out the cb, so the
> first cb will get called for the second chunk.  That can't be
> as-intended, right?
>
> I *think* that some part of this could be done by exposing the
> bufferevent more, but like you I'm a little unclear on the details
> here.  Pausing and unpausing could (I think) be done with
> bufferevent_enable and disable on the user's part; throttling could be
> done with the rate-limiting functionality and the high-water/low-water
> functionality; and did you know that you can have more than one
> callback on an evbuffer?  You can get into lots of fun with that.

I don't know the bufferevent interface well enough to comment
intelligently, but I like watermarks.

For background, my attempt at evhttp flow control. Sad, somehow I
never found the time to polish/test these despite over three years
apparently passing...

intro: http://www.mail-archive.com/libevent-users@xxxxxxxxxx/msg01020.html

latest patch version:
http://www.mail-archive.com/libevent-users@xxxxxxxxxx/msg01022.html

calling code: http://www.slamb.org/projects/foxigniter/ the portions
that need these patches are controlled by
EVHTTP_{WATERMARKS,DETECT_FAILURE} #ifdefs.

My interface was a bit different: persistent watermark-based
fill/drain callbacks, vs Cliff's essentially fixed low watermark of 0
which calls a fill callback which is supplied on the last write.

I think the watermarks are better for a proxy server. Let's say you
get a bunch of data from the outbound connection, fill the inbound
connection's buffer, and pause the outbound connection to prevent more
from accumulating. When do you unpause? With Cliff's patch, you don't
get any more notification until the inbound connection's buffer is
totally empty, so it basically stalls until you resume the outbound
connection and get some data in. With a non-zero low water mark, that
doesn't necessarily happen.

On the other hand, as I write that I'm having second thoughts: maybe
the kernel's socket buffers are sufficient to address this, and you
can even resize those with SO_SNDBUF and SO_RCVBUF as desired. So
maybe Cliff's approach is fine.

>
> But on consideration, this code is pretty darn nice.  I'm hoping other
> evhttp users will have a look at it too, but on the whole I am pretty
> happy about merging it into 2.1.
>
> See also my next mail to this list. :)
>
> --
> Nick
> ***********************************************************************
> To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
> unsubscribe libevent-users    in the body.
>



-- 
Scott Lamb <http://www.slamb.org/>
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.