Mailing List Archive: Re: svn commit: r1891148 - in /httpd/httpd/trunk: include/ap_mmn.h include/util_filter.h modules/proxy/proxy_util.c server/util

On Tue, Jun 29, 2021 at 09:16:21PM -0000, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Jun 29 21:16:21 2021
> New Revision: 1891148
>
> URL: http://svn.apache.org/viewvc?rev=1891148&view=rev
> Log:
> core: Write Completion (WC) bucket type.
>
> 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.

So filters should treat a WC bucket the same as FLUSH in general? And
specifically, filters are broken if they don't? 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.

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.

(Seems we need bucket subclasses...)

Regards, Joe

On Wed, Jun 30, 2021 at 9:42 AM Joe Orton <jorton@redhat.com> wrote:
>
> On Tue, Jun 29, 2021 at 09:16:21PM -0000, ylavic@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.