You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by je...@apache.org on 2002/05/30 09:34:00 UTC
cvs commit: httpd-2.0/modules/proxy mod_proxy.c proxy_http.c
jerenkrantz 02/05/30 00:34:00
Modified: . CHANGES
modules/proxy mod_proxy.c proxy_http.c
Log:
Switch mod_proxy to using the brigade/filter calls directly rather than
the *_client_block calls.
Revision Changes Path
1.803 +2 -2 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.802
retrieving revision 1.803
diff -u -r1.802 -r1.803
--- CHANGES 30 May 2002 05:43:35 -0000 1.802
+++ CHANGES 30 May 2002 07:33:59 -0000 1.803
@@ -9,8 +9,8 @@
Elimiates possible gpfault or garbage title without the -t option.
[William Rowe]
- *) Rewrite mod_cgi and mod_cgid's input handling to use brigades and input
- filters. [Justin Erenkrantz]
+ *) Rewrite mod_cgi, mod_cgid, and mod_proxy input handling to use
+ brigades and input filters. [Justin Erenkrantz]
*) Allow ap_http_filter (HTTP_IN) to return EOS when there is no request
body. [Justin Erenkrantz]
1.84 +0 -3 httpd-2.0/modules/proxy/mod_proxy.c
Index: mod_proxy.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/proxy/mod_proxy.c,v
retrieving revision 1.83
retrieving revision 1.84
diff -u -r1.83 -r1.84
--- mod_proxy.c 17 May 2002 11:24:16 -0000 1.83
+++ mod_proxy.c 30 May 2002 07:33:59 -0000 1.84
@@ -394,9 +394,6 @@
apr_table_set(r->headers_in, "Max-Forwards",
apr_psprintf(r->pool, "%ld", (maxfwd > 0) ? maxfwd : 0));
- if ((rc = ap_setup_client_block(r, REQUEST_CHUNKED_ERROR)))
- return rc;
-
url = r->filename + 6;
p = strchr(url, ':');
if (p == NULL)
1.151 +35 -16 httpd-2.0/modules/proxy/proxy_http.c
Index: proxy_http.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/proxy/proxy_http.c,v
retrieving revision 1.150
retrieving revision 1.151
diff -u -r1.150 -r1.151
--- proxy_http.c 30 May 2002 07:04:45 -0000 1.150
+++ proxy_http.c 30 May 2002 07:33:59 -0000 1.151
@@ -419,12 +419,11 @@
char *url, apr_bucket_brigade *bb,
char *server_portstr) {
conn_rec *c = r->connection;
- char buffer[HUGE_STRING_LEN];
char *buf;
apr_bucket *e;
const apr_array_header_t *headers_in_array;
const apr_table_entry_t *headers_in;
- int counter;
+ int counter, seen_eos;
apr_status_t status;
/*
@@ -620,22 +619,42 @@
}
/* send the request data, if any. */
- if (ap_should_client_block(r)) {
- while ((counter = ap_get_client_block(r, buffer, sizeof(buffer))) > 0) {
- e = apr_bucket_pool_create(buffer, counter, p, c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- e = apr_bucket_flush_create(c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- status = ap_pass_brigade(origin->output_filters, bb);
- if (status != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
- "proxy: pass request data failed to %pI (%s)",
- p_conn->addr, p_conn->name);
- return status;
+ seen_eos = 0;
+ do {
+ status = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,
+ APR_BLOCK_READ, HUGE_STRING_LEN);
+
+ if (status != APR_SUCCESS) {
+ return status;
+ }
+
+ /* If this brigade contain EOS, either stop or remove it. */
+ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
+ /* As a shortcut, if this brigade is simply an EOS bucket,
+ * don't send anything down the filter chain.
+ */
+ if (APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(bb))) {
+ break;
}
- apr_brigade_cleanup(bb);
+
+ /* We can't pass this EOS to the output_filters. */
+ APR_BUCKET_REMOVE(APR_BRIGADE_LAST(bb));
+ seen_eos = 1;
}
- }
+
+ e = apr_bucket_flush_create(c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+
+ status = ap_pass_brigade(origin->output_filters, bb);
+ if (status != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
+ "proxy: pass request data failed to %pI (%s)",
+ p_conn->addr, p_conn->name);
+ return status;
+ }
+ apr_brigade_cleanup(bb);
+ } while (!seen_eos);
+
return APR_SUCCESS;
}
Re: Modules using the input brigade calls directly
Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 30 May 2002, Eli Marmor wrote:
> A small wish "from the field":
>
> Will Justin's stuff be included with the RC3 of 2.0.37?
I'm still procrastinating on tagging RC2. Most of Justin's stuff is
already in, but I'm considering backing it out and waiting until RC3 since
some questions have surfaced. I see no reason why it shouldn't go in the
final 2.0.37 though, assuming we get enough testing time on it between now
and then.
--Cliff
Re: Modules using the input brigade calls directly
Posted by Eli Marmor <ma...@netmask.it>.
A small wish "from the field":
Will Justin's stuff be included with the RC3 of 2.0.37?
--
Eli Marmor
marmor@netmask.it
CTO, Founder
Netmask (El-Mar) Internet Technologies Ltd.
__________________________________________________________
Tel.: +972-9-766-1020 8 Yad-Harutzim St.
Fax.: +972-9-766-1314 P.O.B. 7004
Mobile: +972-50-23-7338 Kfar-Saba 44641, Israel
Re: Modules using the input brigade calls directly
Posted by Eli Marmor <ma...@netmask.it>.
Justin Erenkrantz wrote:
>
> On Thu, May 30, 2002 at 07:34:00AM -0000, jerenkrantz@apache.org wrote:
> > jerenkrantz 02/05/30 00:34:00
> >
> > Modified: . CHANGES
> > modules/proxy mod_proxy.c proxy_http.c
> > Log:
> > Switch mod_proxy to using the brigade/filter calls directly rather than
> > the *_client_block calls.
>
> Okay, with mod_proxy and the cgi variants done, I think I've
> transformed the majority of our uses of ap_*_client_block to
> natively use the input filtering API. (And, mod_deflate's
> new filter follows a similar strategy.)
>
> In case you are interested, here's a summary of what/why I've done:
> ...
> ...
Wow, great! (*)
Finally, Apache completed the migration to the modular model of I/O
filtering...
A historical day that should be remembered! (*)
(*) (The first and the last paragraphs are NOT sarcastic...)
Thanks, Justin (I need no more patches in the core source of Apache...
Now I can do everything cleanly...)
--
Eli Marmor
marmor@netmask.it
CTO, Founder
Netmask (El-Mar) Internet Technologies Ltd.
__________________________________________________________
Tel.: +972-9-766-1020 8 Yad-Harutzim St.
Fax.: +972-9-766-1314 P.O.B. 7004
Mobile: +972-50-23-7338 Kfar-Saba 44641, Israel
Re: Modules using the input brigade calls directly
Posted by Dwayne Miller <dm...@espgroup.net>.
Which module would be a good example for someone who wants to rewrite an
existing
ap_*_client_block
based module?
Tks
Brian Pane wrote:
>On Thu, 2002-05-30 at 01:23, Justin Erenkrantz wrote:
>
>
>
>>- As a side effect, a lot of 8k static buffer allocations have
>> been removed. (I can hear the Netware guys cheering from here.)
>>- This should also greatly reduce the number of copies that the
>> input data should have to go through as it proceeds up to these
>> modules. In the case of mod_proxy, the code is zero-copy from
>> the standpoint of the module - it just gets a brigade from the
>> input and passes it on to the output filter - no copies needed.
>>
>>
>
>Thanks, this is a great improvement.
>
>--Brian
>
>
>
>
Re: Modules using the input brigade calls directly
Posted by Brian Pane <bp...@pacbell.net>.
On Thu, 2002-05-30 at 01:23, Justin Erenkrantz wrote:
> - As a side effect, a lot of 8k static buffer allocations have
> been removed. (I can hear the Netware guys cheering from here.)
> - This should also greatly reduce the number of copies that the
> input data should have to go through as it proceeds up to these
> modules. In the case of mod_proxy, the code is zero-copy from
> the standpoint of the module - it just gets a brigade from the
> input and passes it on to the output filter - no copies needed.
Thanks, this is a great improvement.
--Brian
Modules using the input brigade calls directly
Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, May 30, 2002 at 07:34:00AM -0000, jerenkrantz@apache.org wrote:
> jerenkrantz 02/05/30 00:34:00
>
> Modified: . CHANGES
> modules/proxy mod_proxy.c proxy_http.c
> Log:
> Switch mod_proxy to using the brigade/filter calls directly rather than
> the *_client_block calls.
Okay, with mod_proxy and the cgi variants done, I think I've
transformed the majority of our uses of ap_*_client_block to
natively use the input filtering API. (And, mod_deflate's
new filter follows a similar strategy.)
In case you are interested, here's a summary of what/why I've done:
- These modules will now accept chunked encoding (which will be
dechunked by HTTP_IN). Previously, this was forbidden due to
ap_setup_client_block(). This is probably the major reason why
I did this. I don't believe we should necessarily limit to
only accepting C-L denoted bodies. As I've stated before, if
a module wishes to get the raw chunks, they should remove the
HTTP_IN filter.
- As a side effect, a lot of 8k static buffer allocations have
been removed. (I can hear the Netware guys cheering from here.)
- This should also greatly reduce the number of copies that the
input data should have to go through as it proceeds up to these
modules. In the case of mod_proxy, the code is zero-copy from
the standpoint of the module - it just gets a brigade from the
input and passes it on to the output filter - no copies needed.
- A large amount of code in the get_client_block can be bypassed.
So, this should reduce our input overhead. And, we don't have
to parse the header information twice for each request body
(since we don't call ap_setup_client_block anymore).
- As discussed previously on here (thanks to Greg and Aaron),
HTTP_IN (aka ap_http_filter) will now handle the protocol logic
for you. If there should be no body, it will return EOS. Note
that due to the RFC interpretation, I had to introduce a special
case for proxy's "response" using HTTP_IN so that the response
case with just a "Connection: Close" would be deemed valid
(this case is only valid when dealing with a response body).
Please let me know if you encounter any problems or have any
suggestions. I have tried to test all cases that I could think
of (incl. httpd-test), but it's very possible something slipped
through. -- justin