Hi all, The attached patch addresses a problem with proxy v1.3, and its handling of slow frontends accessing expensive backends.
Currently proxy waits until the frontend client has "eaten up" all the
bytes before closing the backend connection. This causes an unnecessary
delay and chewing of resources when the frontend client takes its time
eating the bytes.
In addition, proxy reads from the backend in hardcoded chunks of 8k in
size. If this figure was larger, proxy would be able to read larger
files in one go - and thus close the backend completely before sending
this larger data block to the frontend.
This patch adds a check in ap_proxy_send_fb() which closes the backend
connection the moment it senses that all the data has been read - but
before attempting to send this final block to the client. This should in
theory solve the problem of the backend lingering around after the
request is finished.
In addition, the size of the proxy in-transit buffer is now set to the
size of the ProxyReceiveBufferSize parameter, with a minimum size of 8k.
This will make it possible for responses larger than 8k to be sent in
one go.
Question: Is it a good idea to overload the ProxyReceiveBufferSize
parameter, or should I create a brand new parameter?
Regards,
Graham
--
-----------------------------------------
[EMAIL PROTECTED] "There's a moon
over Bourbon Street
tonight..."
diff -r -U 3 /home/minfrin/src/apache/pristine/apache-1.3/src/CHANGES
apache-1.3/src/CHANGES
--- /home/minfrin/src/apache/pristine/apache-1.3/src/CHANGES Wed Feb 20 00:17:11
2002
+++ apache-1.3/src/CHANGES Wed Feb 20 23:40:38 2002
@@ -1,5 +1,18 @@
Changes with Apache 1.3.24
+ *) When proxy enabled a slow frontend client to read from an
+ expensive backend server, it would wait until it had delivered
+ the response to the slow frontend client completely before
+ closing the backend connection. The backend connection is now
+ closed as soon as the last byte is read from it, freeing up
+ resources that would have been tied up unnecessarily.
+ [Graham Leggett, Igor Sysoev <[EMAIL PROTECTED]>]
+
+ *) The proxy code read chunks from the backend server in a
+ hardcoded amount of 8k. The existing ProxyReceiveBufferSize
+ parameter has been overloaded to specify the size of this buffer.
+ [Graham Leggett, Igor Sysoev <[EMAIL PROTECTED]>]
+
*) [Security] Prevent invalid client hostnames from appearing in
the log file. If a double-reverse lookup was performed (e.g.,
for an "Allow from .my.domain" directive) but failed, then
diff -r -U 3
/home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/mod_proxy.h
apache-1.3/src/modules/proxy/mod_proxy.h
--- /home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/mod_proxy.h Sun
Feb 3 18:36:44 2002
+++ apache-1.3/src/modules/proxy/mod_proxy.h Wed Feb 20 21:45:38 2002
@@ -295,7 +295,7 @@
char **passwordp, char **hostp, int *port);
const char *ap_proxy_date_canon(pool *p, const char *x);
table *ap_proxy_read_headers(request_rec *r, char *buffer, int size, BUFF *f);
-long int ap_proxy_send_fb(BUFF *f, request_rec *r, cache_req *c, off_t len, int
nowrite);
+long int ap_proxy_send_fb(BUFF *f, request_rec *r, cache_req *c, off_t len, int
+nowrite, size_t recv_buffer_size);
void ap_proxy_write_headers(cache_req *c, const char *respline, table *t);
int ap_proxy_liststr(const char *list, const char *key, char **val);
void ap_proxy_hash(const char *it, char *val, int ndepth, int nlength);
diff -r -U 3
/home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_cache.c
apache-1.3/src/modules/proxy/proxy_cache.c
--- /home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_cache.c
Fri Feb 15 12:43:00 2002
+++ apache-1.3/src/modules/proxy/proxy_cache.c Wed Feb 20 21:35:18 2002
@@ -788,8 +788,7 @@
/* if cache file is being updated */
if (c->origfp) {
ap_proxy_write_headers(c, c->resp_line, c->hdrs);
- ap_proxy_send_fb(c->origfp, r, c, c->len, 1);
- ap_pclosef(r->pool, ap_bfileno(c->origfp, B_WR));
+ ap_proxy_send_fb(c->origfp, r, c, c->len, 1, IOBUFSIZE);
ap_proxy_cache_tidy(c);
}
else
@@ -859,8 +858,7 @@
/* are we updating the cache file? */
if (c->origfp) {
ap_proxy_write_headers(c, c->resp_line, c->hdrs);
- ap_proxy_send_fb(c->origfp, r, c, c->len, 1);
- ap_pclosef(r->pool, ap_bfileno(c->origfp, B_WR));
+ ap_proxy_send_fb(c->origfp, r, c, c->len, 1, IOBUFSIZE);
ap_proxy_cache_tidy(c);
}
else
@@ -893,17 +891,19 @@
/* are we rewriting the cache file? */
if (c->origfp) {
ap_proxy_write_headers(c, c->resp_line, c->hdrs);
- ap_proxy_send_fb(c->origfp, r, c, c->len, r->header_only);
- ap_pclosef(r->pool, ap_bfileno(c->origfp, B_WR));
+ ap_proxy_send_fb(c->origfp, r, c, c->len, r->header_only, IOBUFSIZE);
ap_proxy_cache_tidy(c);
return OK;
}
/* no, we not */
- if (!r->header_only)
- ap_proxy_send_fb(cachefp, r, NULL, c->len, 0);
+ if (!r->header_only) {
+ ap_proxy_send_fb(cachefp, r, NULL, c->len, 0, IOBUFSIZE);
+ }
+ else {
+ ap_pclosef(r->pool, ap_bfileno(cachefp, B_WR));
+ }
- ap_pclosef(r->pool, ap_bfileno(cachefp, B_WR));
return OK;
}
diff -r -U 3
/home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_ftp.c
apache-1.3/src/modules/proxy/proxy_ftp.c
--- /home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_ftp.c Sun
Feb 3 18:36:52 2002
+++ apache-1.3/src/modules/proxy/proxy_ftp.c Wed Feb 20 22:10:01 2002
@@ -264,8 +264,8 @@
static long int send_dir(BUFF *data, request_rec *r, cache_req *c, char *cwd)
{
- char buf[IOBUFSIZE];
- char buf2[IOBUFSIZE];
+ char *buf, *buf2;
+ size_t buf_size;
char *filename;
int searchidx = 0;
char *searchptr = NULL;
@@ -277,6 +277,11 @@
char *dir, *path, *reldir, *site, *type = NULL;
char *basedir = ""; /* By default, path is relative to the $HOME dir */
+ /* create default sized buffers for the stuff below */
+ buf_size = IOBUFSIZE;
+ buf = ap_palloc(r->pool, buf_size);
+ buf2 = ap_palloc(r->pool, buf_size);
+
/* Save "scheme://site" prefix without password */
site = ap_unparse_uri_components(p, &r->parsed_uri,
UNP_OMITPASSWORD|UNP_OMITPATHINFO);
/* ... and path without query args */
@@ -303,7 +308,7 @@
path[n-1] = '\0';
/* print "ftp://host/"; */
- n = ap_snprintf(buf, sizeof(buf), DOCTYPE_HTML_3_2
+ n = ap_snprintf(buf, buf_size, DOCTYPE_HTML_3_2
"<html><head><title>%s%s%s</title>\n"
"<base href=\"%s%s%s\"></head>\n"
"<body><h2>Directory of "
@@ -327,7 +332,7 @@
else
++reldir;
/* print "path/" component */
- ap_snprintf(buf, sizeof(buf), "<a href=\"%s%s/\">%s</a>/",
+ ap_snprintf(buf, buf_size, "<a href=\"%s%s/\">%s</a>/",
basedir,
ap_escape_uri(p, path),
ap_escape_html(p, reldir));
@@ -340,15 +345,15 @@
/* If the caller has determined the current directory, and it differs */
/* from what the client requested, then show the real name */
if (cwd == NULL || strncmp (cwd, path, strlen(cwd)) == 0) {
- ap_snprintf(buf, sizeof(buf), "</h2>\n<hr /><pre>");
+ ap_snprintf(buf, buf_size, "</h2>\n<hr /><pre>");
} else {
- ap_snprintf(buf, sizeof(buf), "</h2>\n(%s)\n<hr /><pre>",
+ ap_snprintf(buf, buf_size, "</h2>\n(%s)\n<hr /><pre>",
ap_escape_html(p, cwd));
}
total_bytes_sent += ap_proxy_bputs2(buf, con->client, c);
while (!con->aborted) {
- n = ap_bgets(buf, sizeof buf, data);
+ n = ap_bgets(buf, buf_size, data);
if (n == -1) { /* input error */
if (c != NULL) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, c->req,
@@ -375,12 +380,12 @@
if (filename != buf)
*(filename++) = '\0';
*(link_ptr++) = '\0';
- ap_snprintf(buf2, sizeof(buf2), "%s <a href=\"%s\">%s %s</a>\n",
+ ap_snprintf(buf2, buf_size, "%s <a href=\"%s\">%s %s</a>\n",
ap_escape_html(p, buf),
ap_escape_uri(p,filename),
ap_escape_html(p, filename),
ap_escape_html(p, link_ptr));
- ap_cpystrn(buf, buf2, sizeof(buf));
+ ap_cpystrn(buf, buf2, buf_size);
n = strlen(buf);
}
/* Handle unix style or DOS style directory */
@@ -410,23 +415,23 @@
/* Special handling for '.' and '..': append slash to link */
if (!strcmp(filename, ".") || !strcmp(filename, "..") || buf[0] == 'd') {
- ap_snprintf(buf2, sizeof(buf2), "%s <a href=\"%s/\">%s</a>\n",
+ ap_snprintf(buf2, buf_size, "%s <a href=\"%s/\">%s</a>\n",
ap_escape_html(p, buf), ap_escape_uri(p,filename),
ap_escape_html(p, filename));
}
else {
- ap_snprintf(buf2, sizeof(buf2), "%s <a href=\"%s\">%s</a>\n",
+ ap_snprintf(buf2, buf_size, "%s <a href=\"%s\">%s</a>\n",
ap_escape_html(p, buf),
ap_escape_uri(p,filename),
ap_escape_html(p, filename));
}
- ap_cpystrn(buf, buf2, sizeof(buf));
+ ap_cpystrn(buf, buf2, buf_size);
n = strlen(buf);
}
/* else??? What about other OS's output formats? */
else {
strcat(buf, "\n"); /* re-append the newline char */
- ap_cpystrn(buf, ap_escape_html(p, buf), sizeof(buf));
+ ap_cpystrn(buf, ap_escape_html(p, buf), buf_size);
}
total_bytes_sent += ap_proxy_bputs2(buf, con->client, c);
@@ -438,6 +443,8 @@
total_bytes_sent += ap_proxy_bputs2(ap_psignature("", r), con->client, c);
total_bytes_sent += ap_proxy_bputs2("</body></html>\n", con->client, c);
+ ap_bclose(data);
+
ap_bflush(con->client);
return total_bytes_sent;
@@ -1341,10 +1348,12 @@
/* we need to set this for ap_proxy_send_fb()... */
if (c != NULL)
c->cache_completion = 0;
- ap_proxy_send_fb(data, r, c, -1, 0);
- } else
+ ap_proxy_send_fb(data, r, c, -1, 0, conf->recv_buffer_size);
+ }
+ else {
send_dir(data, r, c, cwd);
- ap_bclose(data);
+ }
+ /* ap_proxy_send_fb() closes the socket */
data = NULL;
dsock = -1;
diff -r -U 3
/home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_http.c
apache-1.3/src/modules/proxy/proxy_http.c
--- /home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_http.c Fri
Feb 15 12:43:56 2002
+++ apache-1.3/src/modules/proxy/proxy_http.c Wed Feb 20 21:09:10 2002
@@ -582,12 +582,12 @@
* content length is not known. We need to make 100% sure c->len is always
* set correctly before we get here to correctly do keepalive.
*/
- ap_proxy_send_fb(f, r, c, c->len, 0);
+ ap_proxy_send_fb(f, r, c, c->len, 0, conf->recv_buffer_size);
}
- ap_proxy_cache_tidy(c);
+ /* ap_proxy_send_fb() closes the socket f for us */
- ap_bclose(f);
+ ap_proxy_cache_tidy(c);
ap_proxy_garbage_coll(r);
return OK;
diff -r -U 3
/home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_util.c
apache-1.3/src/modules/proxy/proxy_util.c
--- /home/minfrin/src/apache/pristine/apache-1.3/src/modules/proxy/proxy_util.c Fri
Feb 15 12:44:00 2002
+++ apache-1.3/src/modules/proxy/proxy_util.c Wed Feb 20 21:57:12 2002
@@ -497,15 +497,21 @@
* - r->connection->client, if nowrite == 0
*/
-long int ap_proxy_send_fb(BUFF *f, request_rec *r, cache_req *c, off_t len, int
nowrite)
+long int ap_proxy_send_fb(BUFF *f, request_rec *r, cache_req *c, off_t len, int
+nowrite, size_t recv_buffer_size)
{
int ok;
- char buf[IOBUFSIZE];
+ char *buf;
+ size_t buf_size;
long total_bytes_rcvd;
register int n, o, w;
conn_rec *con = r->connection;
int alternate_timeouts = 1; /* 1 if we alternate between soft & hard timeouts */
+ /* allocate a buffer to store the bytes in */
+ /* make sure it is at least IOBUFSIZE, as recv_buffer_size may be zero for system
+default */
+ buf_size = MAX(recv_buffer_size, IOBUFSIZE);
+ buf = ap_palloc(r->pool, buf_size);
+
total_bytes_rcvd = 0;
if (c != NULL)
c->written = 0;
@@ -554,10 +560,10 @@
/* Read block from server */
if (-1 == len) {
- n = ap_bread(f, buf, IOBUFSIZE);
+ n = ap_bread(f, buf, buf_size);
}
else {
- n = ap_bread(f, buf, MIN(IOBUFSIZE, len - total_bytes_rcvd));
+ n = ap_bread(f, buf, MIN(buf_size, len - total_bytes_rcvd));
}
if (alternate_timeouts)
@@ -578,6 +584,18 @@
o = 0;
total_bytes_rcvd += n;
+ /* if we've received everything... */
+ /* in the case of slow frontends and expensive backends,
+ * we want to avoid leaving a backend connection hanging
+ * while the frontend takes it's time to absorb the bytes.
+ * so: if we just read the last block, we close the backend
+ * connection now instead of later - it's no longer needed.
+ */
+ if (total_bytes_rcvd == len) {
+ ap_bclose(f);
+ f = NULL;
+ }
+
/* Write to cache first. */
/*@@@ XXX FIXME: Assuming that writing the cache file won't time out?!!? */
if (c != NULL && c->fp != NULL) {
@@ -634,8 +652,14 @@
} /* loop and ap_bread while "ok" */
- if (!con->aborted)
+ /* if the backend connection is still open, close it */
+ if (f) {
+ ap_bclose(f);
+ }
+
+ if (!con->aborted) {
ap_bflush(con->client);
+ }
ap_kill_timeout(r);
return total_bytes_rcvd;
smime.p7s
Description: S/MIME Cryptographic Signature
