httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: [Patch] Async write completion for the full connection filter stack
Date Wed, 10 Sep 2014 16:07:10 GMT
On 09 Sep 2014, at 9:19 PM, Plüm, Rüdiger, Vodafone Group <ruediger.pluem@vodafone.com>
wrote:

>> I spent a lot of time going down this path of having a dedicated
>> metabucket, and quickly got bogged down in complexity. The key problem
>> was "what does a filter actually do when you get one", it was unclear
> 
> Don't we have the same problem with an empty brigade? Some filters are not going
> to handle it as we expect. Hence the additional logic in ap_pass_brigade.
> I guess the minimum behavior we need to get from every filter is to ignore and pass on.

We don’t have the same problem with an empty brigade, no. Empty brigades “fail safe”
in that a naive filter will simply try and loop past the brigade, exiting immediately having
done nothing.

Meta buckets are inherently unsafe in comparison, because if a module doesn’t pass the meta
bucket downstream for whatever buggy reason, the server will start to spin. We could use compensation
code to stop this, but the compensation code would be complex. Testing for an empty brigade
is cheap and trivial, looping through a brigade to search for NOOP buckets so as to trigger
bypass of the compensation code is not so trivial.

>> and it made my head bleed. That makes life hard for module authors and
>> that is bad. As I recall there were also broken filters out there that
>> only knew about FLUSH and EOS buckets (eg ap_http_chunk_filter()).
> 
> We already have additional metabuckets like error buckets or EOR.
> So I don't see an issue creating a new one. Any filter not passing a meta bucket
> that is does not understand or even try to process it is simply broken. 

Agreed that such a filter is broken, but such filters exist (the chunk filter being one example,
if I looked harder I would find others) and we need to work within this limitation.

Having thought long and hard about this, giving filters an opportunity to write has nothing
to do with either data or metadata, we just want to give the filter an opportunity to write.
“Dear filter, please wake up and if you’ve setaside data please write it, otherwise do
nothing, thanks”. Empty brigade means “do nothing new”.

>> The problem we're trying to solve is one of starvation - no filters can
>> set aside data for later (except core via the NULL hack), because there
>> is no guarantee that they'll ever be called again later. You have to
>> write it now, or potentially write it never. The start of the solution
>> is ensure filters aren't starved: if you have data in the output filters
>> - and obviously you have no idea which filters have setaside data - you
>> need a way to wake them all up. The simplest and least disruptive way is
>> to pass them all an empty brigade, job done. We've got precedent for
>> this - we've been sending NULL to the core filter to achieve the same
> 
> But this is *our* filter and it will not hit any custom filters. So we can
> do this kind of hacky game here.
> 
>> thing, we want something that works with any filter.
> 
> Yes, and this is the reason why I still believe a meta bucket is better.

I suspect it is not completely clear how the patch works.

Previously, in the core, we handled write completion by passing NULL to the last filter in
the chain, the core output filter. This worked fine for simple requests, but as soon as another
filter was involved - primarily mod_ssl - there was no write completion. The work of mod_ssl
had to be completed in its entirety before write completion would kick in, and by that point
it was too late to be useful.

The fix for this problem starts by including the entire connection filter chain into write
completion, not just the last filter. That means every connection filter both httpd internal
and external can take advantage of write completion, not just the core filter.

So what do you pass during this write completion phase? Obviously not data, you’ve finished
passing the data already when the handler exited. We can’t pass NULL like the core filter
has, because all filters would just segfault. This leaves an obvious choice - an empty brigade.

We have a new problem - not all filters pass data downstream for one of many reasons. Maybe
they’re buggy. Maybe they’re buffering data. Maybe that’s what the filter believes it
was asked to do. So we must compensate for this by “waking up” filters downstream of the
filter that didn’t write. All we need to do to those filters is wake them up, that’s it.

At this point we’re still not done - we need cooperation from filters in order to support
write completion, a filter must yield and setaside the data it was given to be called back
later, just like the core does. This gives other requests an opportunity to get CPU time.
It turns out the algorithm to achieve this yield is built into the core filter, so the next
step is to generalise the algorithm and make it available to any filter, starting with mod_ssl.

The attached patch does that.

Regards,
Graham
—

Mime
View raw message