You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <ju...@erenkrantz.com> on 2003/08/31 18:39:39 UTC

[PROPOSAL] Remove ap_*_client_block in 2.1 series

I'd like to axe ap_{setup|should|get}_block in future releases (i.e. 2.1+).

The problems with ap_*_client_block are:

- Errors can not be returned to the client successfully (only -1 allowed)
- Inconsistencies because the filtering API returns EOSes inlined with data
  (ap_get_client_block requires a separate call for EOS return)
- Duplicate processing of headers to support ap_should_client_block.
  (We'll be able to shrink request_rec a bit.)

We've supported it in 2.0 alongside the new ap_get_brigade API.  I've got a 
fix for Stas's showstopper in the tree for 2.1 that I proposed for 2.0.  But, 
I'd like to yank it for 2.1 because I don't believe we should be supporting 
this API moving forward.

If no one speaks up, I'll yank it in about a week or so.  The only module left 
in our tree that still supports it is mod_isapi.  I don't have access to a 
Win32 box, but I *could* try to take a blind pass at making it call 
ap_get_brigade instead.  -- justin

Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Greg Stein <gs...@lyra.org>.
On Sun, Aug 31, 2003 at 09:39:39AM -0700, Justin Erenkrantz wrote:
> I'd like to axe ap_{setup|should|get}_block in future releases (i.e. 2.1+).

+1 ... absolutely zero pushback from this camp :-)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Sep 10, 2003 at 10:27:42AM -0400, gregames@remulak.net wrote:
> Greg Stein wrote:
> 
> > ap_rput* should be torched as well. 
> 
> what about simplicity?  how many lines of code are required for an alternative?

Yes, the ap_r* functions are a bit simpler for the module author, but
definitely cause undue complexity within the interface.

The ap_f* functions are pretty much:

{
  brigade bb = apr_brigade_create(pool);
  
  ap_fputs(r->output_filters, bb, "hello there");
  ap_fwrite(r->output_filters, bb, buf, len);
  ...
  
  return ap_pass_brigade(r->output_filters, bb);
}

IOW, before a block of writes, create a working brigade. At the end, shove
it down the filter stack.

Of course, you also want to ensure that you brigade creation is bounded on
a per-request basis. You wouldn't want to call the function above a
million times :-) In the SVN code, we create a single brigade at the
control point where we begin to generate the response. We use it all
throughout by passing the bb around, and then pass it at the end of the
response function. It is actually quite clean. (and usually
r->output_filters is already deref'd to an ap_filter_t named 'output').

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by gr...@remulak.net.
Greg Stein wrote:

> ap_rput* should be torched as well. 

what about simplicity?  how many lines of code are required for an alternative?

Greg


Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Sep 01, 2003 at 07:35:26PM -0700, Justin Erenkrantz wrote:
>...
> > What about that other inconsistency: the difference between
> > ap_rputs/family in Handlers and ap_fputs/etc in Filters?  The subtle
> > difference to the arguments to these is - erm - annoying!
> 
> ISTR there was a vote before my time on this issue.  Anyone?  -- justin

ap_rput* should be torched as well. The filter functions should be used
instead with r->output_filters.

Tossing the ap_rput* functions would also get rid of the OUTPUT_WRITE
filter stuff, which would be quite nice.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Bojan Smojver <bo...@rexursive.com>.
On Tue, 2003-09-02 at 12:35, Justin Erenkrantz wrote:

> The point of making a number of 2.1 releases before 2.2 is to allow time for 
> module writers to adjust to the new API in whatever way they need to.  IMHO, 
> this was our mistake last time - we released a 2.0 GA release before we had 
> the API 'finalized,' and didn't allow gestation for module writers to adapt to 
> the new 2.0 API.

As a future IAMV (he, he, just coined that - Independent Apache Module
Vendor), I fully agree with this. Thanks fellows, keep doing a great
job!

-- 
Bojan


Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Sunday, August 31, 2003 11:27 PM +0100 Nick Kew <ni...@webthing.com> wrote:

> AIUI 2.1 is supposed to be a minimal change from 2.0.  This proposal
> will break (an unknown number of) 2.0 modules.  Last time I looked,

No one ever said 2.2 is supposed to be a minimal change.  =)

The point of making a number of 2.1 releases before 2.2 is to allow time for 
module writers to adjust to the new API in whatever way they need to.  IMHO, 
this was our mistake last time - we released a 2.0 GA release before we had 
the API 'finalized,' and didn't allow gestation for module writers to adapt to 
the new 2.0 API.

Once we do a 2.1 RC, the API will be fixed, and there will be a few months (?) 
for module writers to get ready.  (Compare with Linux 2.4 and 2.6 kernel 
drivers.)

> it was the only *documented* API for reading input data.  IMO better
> to mark it as deprecated, but leave it in until something is
> *expected* to break third-party modules (like 1.3->2.0 did).

ap_get_brigade() is the recommended approach and has been available for the 
entire 2.0 series and is fully documented.  We haven't been explicit that 
ap_*_client_block should be removed, but I've posted on this list several 
times how to take an old module using the ap_*_client_block API and switch it 
to equivalent ap_get_brigade logic.

Regardless, I would expect in a 2.1 RC or 2.2 release, we would have an 
'upgrading' guide to explain how to switch code using ap_*_client_block to 
ap_get_brigade.

Note, in this particular case, if you switch your module to using 
ap_get_brigade(), you'll be backwards source-compatible with 2.0 and 2.1/2.2. 
And, 2.0 will have ap_*_client_block forever.  ;-)

> What about that other inconsistency: the difference between
> ap_rputs/family in Handlers and ap_fputs/etc in Filters?  The subtle
> difference to the arguments to these is - erm - annoying!

ISTR there was a vote before my time on this issue.  Anyone?  -- justin

Re: [PROPOSAL] Remove ap_*_client_block in 2.1 series

Posted by Nick Kew <ni...@webthing.com>.
On Sun, 31 Aug 2003, Justin Erenkrantz wrote:

> I'd like to axe ap_{setup|should|get}_block in future releases (i.e. 2.1+).
>
> The problems with ap_*_client_block are:
>
> - Errors can not be returned to the client successfully (only -1 allowed)
> - Inconsistencies because the filtering API returns EOSes inlined with data
>   (ap_get_client_block requires a separate call for EOS return)
> - Duplicate processing of headers to support ap_should_client_block.
>   (We'll be able to shrink request_rec a bit.)
>
> We've supported it in 2.0 alongside the new ap_get_brigade API.  I've got a
> fix for Stas's showstopper in the tree for 2.1 that I proposed for 2.0.  But,
> I'd like to yank it for 2.1 because I don't believe we should be supporting
> this API moving forward.

AIUI 2.1 is supposed to be a minimal change from 2.0.  This proposal
will break (an unknown number of) 2.0 modules.  Last time I looked,
it was the only *documented* API for reading input data.  IMO better
to mark it as deprecated, but leave it in until something is
*expected* to break third-party modules (like 1.3->2.0 did).

OTOH I agree with your points about its weaknesses.

What about that other inconsistency: the difference between
ap_rputs/family in Handlers and ap_fputs/etc in Filters?  The subtle
difference to the arguments to these is - erm - annoying!

-- 
Nick Kew