Right,
Here's my latest patch. I've broken out the parsing/request initialisation
code from commReadRequest() into a seperate function which I can then
call from keepaliveNextRequest(). Please review and comment.
I've tested it locally and it seems to work just fine.
There is, however, a fun massive memory abuse I'm seeing when I'm
stressing out squid with mass recursive FTP transfers, along with
loads and loads of ISOs.
http://cyllene.uwa.edu.au/~adrian/bt .. have a look. That particular
node had about 37,000 entries.. Squid took up a good 1.2gig of RAM.
It _did_ recover after about 15 minutes but the memory was so
fragmented I needed to restart to get any decent performance..
Adrian
Index: client_side.cc
===================================================================
RCS file: /server/cvs-server/squid/squid3/src/client_side.cc,v
retrieving revision 1.668
diff -u -r1.668 client_side.cc
--- client_side.cc 22 Dec 2003 10:28:49 -0000 1.668
+++ client_side.cc 24 Feb 2004 08:30:59 -0000
@@ -119,6 +119,8 @@
static CWCB clientWriteComplete;
static IOWCB clientWriteBodyComplete;
static IOCB clientReadRequest;
+static bool clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read);
+static void clientAfterReadingRequests(int fd, ConnStateData::Pointer &conn, int
do_next_read);
static PF connStateFree;
static PF requestTimeout;
static PF clientLifetimeTimeout;
@@ -1307,10 +1309,30 @@
ClientSocketContext::keepaliveNextRequest()
{
ConnStateData::Pointer conn = http->getConn();
+ bool do_next_read = false;
debug(33, 3) ("ClientSocketContext::keepaliveNextRequest: FD %d\n", conn->fd);
connIsFinished();
+ if (Config.onoff.half_closed_clients && clientParseRequest(conn, do_next_read)) {
+ debug(33, 3) ("clientSocketContext::keepaliveNextRequest: FD %d: parsing
next request from buffer\n", conn->fd);
+ clientAfterReadingRequests(conn->fd, conn, do_next_read);
+ return;
+ }
+
+ /*
+ * Either we need to kick-start another read or, if we have
+ * a half-closed connection, kill it after the last request.
+ * This saves waiting for half-closed connections to finished being
+ * half-closed _AND_ then, sometimes, spending "Timeout" time in
+ * the keepalive "Waiting for next request" state.
+ */
+ if (commIsHalfClosed(conn->fd) && (conn->getConcurrentRequestCount() == 0)) {
+ debug(33, 3) ("ClientSocketContext::keepaliveNextRequest: half-closed client
with no pending requests, closing\n");
+ comm_close(conn->fd);
+ return;
+ }
+
ClientSocketContext::Pointer deferredRequest;
if ((deferredRequest = conn->getCurrentContext()).getRaw() == NULL)
@@ -2275,15 +2297,82 @@
return result;
}
+static bool
+clientParseRequest(ConnStateData::Pointer conn, bool &do_next_read)
+{
+ method_t method;
+ char *prefix = NULL;
+ ClientSocketContext *context;
+
+ /* XXX: if we read *exactly* two requests, and the client sends no more,
+ * if pipelined requests are off, we will *never* parse and insert the
+ * second. the corner condition is due to the parsing being tied to the
+ * read, not the presence of data in the buffer.
+ */
+ while (conn->in.notYetUsed > 0 && conn->body.size_left == 0) {
+ size_t req_line_sz;
+ connStripBufferWhitespace (conn);
+
+ if (conn->in.notYetUsed == 0) {
+ clientAfterReadingRequests(conn->fd, conn, do_next_read);
+ return false;
+ }
+
+ /* Limit the number of concurrent requests to 2 */
+ if (!connOkToAddRequest(conn)) {
+ return false;
+ }
+
+ /* Should not be needed anymore */
+ /* Terminate the string */
+ conn->in.buf[conn->in.notYetUsed] = '\0';
+
+ /* Process request */
+ context = parseHttpRequest(conn, &method, &prefix, &req_line_sz);
+
+ /* partial or incomplete request */
+ if (!context) {
+ safe_free(prefix);
+
+ if (!connKeepReadingIncompleteRequest(conn))
+ connCancelIncompleteRequests(conn);
+
+ break; /* conn->in.notYetUsed > 0 && conn->body.size_left == 0 */
+ }
+
+ /* status -1 or 1 */
+ if (context) {
+ commSetTimeout(conn->fd, Config.Timeout.lifetime, clientLifetimeTimeout,
+ context->http);
+
+ clientProcessRequest(conn, context, method, prefix, req_line_sz);
+
+ safe_free(prefix);
+
+ if (context->mayUseConnection()) {
+ debug (33, 3) ("clientReadRequest: Not reading, as this request may
need the connection\n");
+ do_next_read = 0;
+ break;
+ }
+
+ if (!conn->flags.readMoreRequests) {
+ conn->flags.readMoreRequests = 1;
+ break;
+ }
+
+ continue; /* while offset > 0 && body.size_left == 0 */
+ }
+ } /* while offset > 0 && conn->body.size_left == 0 */
+
+ return true;
+}
+
static void
clientReadRequest(int fd, char *buf, size_t size, comm_err_t flag, int xerrno,
void *data)
{
ConnStateData::Pointer conn ((ConnStateData *)data);
conn->reading(false);
- method_t method;
- char *prefix = NULL;
- ClientSocketContext *context;
bool do_next_read = 1; /* the default _is_ to read data! - adrian */
assert (fd == conn->fd);
@@ -2347,68 +2436,8 @@
if (conn->getConcurrentRequestCount() == 0)
fd_note(conn->fd, "Reading next request");
- /* XXX: if we read *exactly* two requests, and the client sends no more,
- * if pipelined requests are off, we will *never* parse and insert the
- * second. the corner condition is due to the parsing being tied to the
- * read, not the presence of data in the buffer.
- */
- while (conn->in.notYetUsed > 0 && conn->body.size_left == 0) {
- size_t req_line_sz;
- connStripBufferWhitespace (conn);
-
- if (conn->in.notYetUsed == 0) {
- clientAfterReadingRequests(fd, conn, do_next_read);
- return;
- }
-
- /* Limit the number of concurrent requests to 2 */
- if (!connOkToAddRequest(conn)) {
- return;
- }
-
- /* Should not be needed anymore */
- /* Terminate the string */
- conn->in.buf[conn->in.notYetUsed] = '\0';
-
- /* Process request */
- context = parseHttpRequest(conn,
- &method, &prefix, &req_line_sz);
-
- /* partial or incomplete request */
- if (!context) {
- safe_free(prefix);
-
- if (!connKeepReadingIncompleteRequest(conn))
- connCancelIncompleteRequests(conn);
-
- break; /* conn->in.notYetUsed > 0 && conn->body.size_left == 0 */
- }
-
- /* status -1 or 1 */
- if (context) {
- commSetTimeout(fd, Config.Timeout.lifetime, clientLifetimeTimeout,
- context->http);
-
- clientProcessRequest(conn, context, method, prefix, req_line_sz);
-
- safe_free(prefix);
-
- if (context->mayUseConnection()) {
- debug (33, 3) ("clientReadRequest: Not reading, as this request may
need the connection\n");
- do_next_read = 0;
- break;
- }
-
- if (!conn->flags.readMoreRequests) {
- conn->flags.readMoreRequests = 1;
- break;
- }
-
- continue; /* while offset > 0 && body.size_left == 0 */
- }
- } /* while offset > 0 && conn->body.size_left == 0 */
-
- clientAfterReadingRequests(fd, conn, do_next_read);
+ if (clientParseRequest(conn, do_next_read))
+ clientAfterReadingRequests(fd, conn, do_next_read);
}
/* file_read like function, for reading body content */
Index: comm.cc
===================================================================
RCS file: /server/cvs-server/squid/squid3/src/comm.cc,v
retrieving revision 1.392
diff -u -r1.392 comm.cc
--- comm.cc 18 Feb 2004 01:58:59 -0000 1.392
+++ comm.cc 24 Feb 2004 08:30:59 -0000
@@ -2560,6 +2560,19 @@
fdc_table[fd].half_closed = true;
}
+int commIsHalfClosed(int fd)
+{
+ assert(fdc_table[fd].active == 1);
+ return fdc_table[fd].half_closed;
+}
+
+void
+commCheckHalfClosed(void *data)
+{
+ AbortChecker::Instance().doIOLoop();
+ eventAdd("commCheckHalfClosed", commCheckHalfClosed, NULL, 1.0, false);
+}
+
AbortChecker &AbortChecker::Instance() {return Instance_;}
AbortChecker AbortChecker::Instance_;
@@ -2601,22 +2614,8 @@
#include "splay.h"
void
AbortChecker::doIOLoop() {
- if (checking) {
- /*
- fds->walk(RemoveCheck, this);
- */
- checking = false;
- return;
- }
-
- if (lastCheck >= squid_curtime)
- return;
-
+ fds->walk(RemoveCheck, this);
fds->walk(AddCheck, this);
-
- checking = true;
-
- lastCheck = squid_curtime;
}
void
Index: main.cc
===================================================================
RCS file: /server/cvs-server/squid/squid3/src/main.cc,v
retrieving revision 1.389
diff -u -r1.389 main.cc
--- main.cc 19 Sep 2003 07:06:19 -0000 1.389
+++ main.cc 24 Feb 2004 08:30:59 -0000
@@ -44,6 +44,7 @@
#include "ACL.h"
#include "htcp.h"
#include "StoreFileSystem.h"
+#include "comm.h"
#if USE_WIN32_SERVICE
@@ -837,6 +838,7 @@
#endif
eventAdd("memPoolCleanIdlePools", Mem::CleanIdlePools, NULL, 15.0, 1);
+ eventAdd("commCheckHalfClosed", commCheckHalfClosed, NULL, 1.0, false);
}
configured_once = 1;
Index: comm.h
===================================================================
RCS file: /server/cvs-server/squid/squid3/src/comm.h,v
retrieving revision 1.20
diff -u -r1.20 comm.h
--- comm.h 15 Aug 2003 13:06:34 -0000 1.20
+++ comm.h 24 Feb 2004 08:31:00 -0000
@@ -30,6 +30,8 @@
extern void comm_write(int s, const char *buf, size_t len, IOWCB *callback, void
*callback_data);
#include "Store.h"
extern void commMarkHalfClosed(int);
+extern int commIsHalfClosed(int);
+extern void commCheckHalfClosed(void *);
extern bool comm_has_incomplete_write(int);
/* Where should this belong? */
