Fix socket accounting for TCP accept() (#854)

The incoming_sockets_accepted counter is used by Comm I/O modules to
count the number of successfully accept()ed FDs during a single
select(2) (or equivalent) scan of ready descriptors. For TCP
connections, the official code incremented that counter in the async
TcpAcceptor callbacks. Those callbacks run outside the counting Comm I/O
context, rendering counter increments useless and resulting in
under-reporting of actual acceptance activity.

Also, some successfully accepted TCP connections fire no callbacks, so
it is conceptually wrong to count accepted sockets in callbacks,
regardless of the timing problems.

diff --git a/src/client_side.cc b/src/client_side.cc
index ef4baff8c9b3fafd54cb87240a03c5fa18708613..004f9c907d37cfa1fad0d4343d6a34f9bdafa1d8 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2360,8 +2360,6 @@ httpAccept(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled)
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
 
-    ++incoming_sockets_accepted;
-
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Http::NewServer(xact);
     AsyncJob::Start(srv); // usually async-calls readSomeData()
@@ -2565,7 +2563,6 @@ httpsAccept(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled) {
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
     }
-    ++incoming_sockets_accepted;
 
     // Socket is ready, setup the connection manager to start using it
     auto *srv = Https::NewServer(xact);

diff --git a/src/comm/TcpAcceptor.cc b/src/comm/TcpAcceptor.cc
index 9da12e526fcc339bf0647c012ce7e634dcd08896..17639607f72b0d74d34b85cc4339f47a73172d26 100644 (file)
--- a/src/comm/TcpAcceptor.cc
+++ b/src/comm/TcpAcceptor.cc
@@ -369,6 +369,7 @@ Comm::TcpAcceptor::oldAccept(Comm::ConnectionPointer &details)
     }
 
     Must(sock >= 0);
+    ++incoming_sockets_accepted;
 
     // Sync with Comm ASAP so that abandoned details can properly close().
     // XXX : these are not all HTTP requests. use a note about type and ip:port details->

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 533cb0a89b34458f28072d231c2b54f639d05df4..8b015f72d10c89838d782e9e451863ef4d88efc7 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -257,8 +257,6 @@ Ftp::Server::AcceptCtrlConnection(const CommAcceptCbParams &params)
     if (s->tcp_keepalive.enabled)
         commSetTcpKeepalive(params.conn->fd, s->tcp_keepalive.idle, s->tcp_keepalive.interval, s->tcp_keepalive.timeout);
 
-    ++incoming_sockets_accepted;
-
     AsyncJob::Start(new Server(xact));
 }
 
@@ -404,7 +402,6 @@ Ftp::Server::acceptDataConnection(const CommAcceptCbParams &params)
 
     debugs(33, 4, "accepted " << params.conn);
     fd_note(params.conn->fd, "passive client ftp data");
-    ++incoming_sockets_accepted;
 
     if (!clientConnection) {
         debugs(33, 5, "late data connection?");