On Wed, Jun 30, 2021 at 9:42 AM Joe Orton <jor...@redhat.com> wrote: > > On Tue, Jun 29, 2021 at 09:16:21PM -0000, yla...@apache.org wrote: > > > > A WC bucket is meant to prevent buffering/coalescing filters from retaining > > data, but unlike a FLUSH bucket it won't cause the core output filter to > > block trying to flush anything before. > > Interesting. I think it would be helpful to have the semantics of this > bucket type described in the header as well.
If I'm not mistaken, a simple way to describe FLUSH semantics is: FLUSH buckets can't be retained/setaside by a filter. For WC buckets, it would be: WC buckets can't be retained/setaside by a filter UNLESS the next filters still have pending data after passing them a trailing WC bucket. I think this means for most filters that yes, they are the same. A better idea for the description of the semantics in ap_filter.h? > > So filters should treat a WC bucket the same as FLUSH in general? And > specifically, filters are broken if they don't? Yes (almost then), and yes because WC buckets would break pollers (MPM event, proxy_tunnel) that wait for write completion. > It seems like an > accident that this makes mod_ssl's coalesce filter flush, merely because > it will flush for *any* metadata bucket type, but it didn't have to be > designed that way. Yeah, that's why I started simple with a new META bucket type, that was enough for the SSL coalesce case. I think it's useful for async in general where we want to never block, on the ap_core_output_filter() side we possibly want to disable FlushMaxThreshold when a WC is found, for instance. As for the SSL coalesce filter, it possibly could be a core filter on its own (at AP_FTYPE_CONNECTION + 4 still) so that we could apply a FlushMinThreshold for both TLS and plain connections. > > If so I wonder if it wouldn't be better to overload FLUSH for this, e.g. > by having a FLUSH bucket with a non-NULL ->data pointer or something? > The core knows it is special but everywhere else treats as FLUSH. That's a great idea, let me try that. > > (Seems we need bucket subclasses...) Oh, I thought we had that already, with (poly)morphism and so on :) Thanks; Yann.
