From: Amos Jeffries Date: Sun, 19 Jul 2015 13:41:08 +0000 (-0700) Subject: Improve handling of client connections on shutdown X-Git-Tag: merge-candidate-3-v1~38^2~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d442618d289f20caa1def027225417cd0de3b078;p=thirdparty%2Fsquid.git Improve handling of client connections on shutdown When Squid which are processing a lot of traffic, using persistent client connections, or dealing with long duration requests are shutdown they can exit with a lot of connections still open. The shutdown_lifetime directive exists to allow time for existing transactions to complete, but this is not always possible and has no effect on idle connections. The result is a large dump of aborted FD entries being logged as the TCP sockets get abruptly reset. Potentially active transactions cache objects being "corrupted" in the process. Makes ConnStateData and its children implement Runner API callbacks to receive signals about Squid shutdown. Which allows their close() handlers to be run properly and make use of AsyncCalls API. Idle client connections are closed immediately on the startShutdown() signal, so their closure CPU cycles happens during the shutdown grace period. An extra 0-delay event step is added to SignalEngine shutdown sequence with a new Runner registry hook 'endingShutdown' is added to signal that the shutdown_lifetime grace period is over for closure of active transactions. All network FD sockets should be considered unusable for read()/write() at that point since close handlers may have already been scheduled by other Runners. AsyncCall's may still be scheduled to release resources. Also adds a DeregisterRunner() API action to remove Runners dynamically from the registered set. The Squid shutdown sequence is now: * shutdown signal received: - listening sockets closed - idle client connections closed * shutdown grace period ends: - remaining client connections closed * shutdown finishes: - main signal and Async loop halted - all memory free'd Server connections which are PINNED or in active use during the endingShutdown execution will be closed cleanly as a side-effect of the client closures. Otherwise there is no change (yet) to server connections or other FD sockets behaviour on shutdown. --- diff --git a/src/base/RunnersRegistry.cc b/src/base/RunnersRegistry.cc index af7775e9e8..fd7a01a158 100644 --- a/src/base/RunnersRegistry.cc +++ b/src/base/RunnersRegistry.cc @@ -32,6 +32,14 @@ RegisterRunner(RegisteredRunner *rr) return runners.size(); } +int +DeregisterRunner(RegisteredRunner *rr) +{ + Runners &runners = GetRunners(); + runners.erase(rr); + return runners.size(); +} + void RunRegistered(const RegisteredRunner::Method &m) { diff --git a/src/base/RunnersRegistry.h b/src/base/RunnersRegistry.h index b21c56f8f2..11312b1eb3 100644 --- a/src/base/RunnersRegistry.h +++ b/src/base/RunnersRegistry.h @@ -68,6 +68,12 @@ public: /// Meant for cleanup and state saving that may require other modules. virtual void startShutdown() {} + /// Called after shutdown_lifetime grace period ends and before stopping + /// the main loop. At least one main loop iteration is guaranteed after + /// this call. + /// Meant for cleanup and state saving that may require other modules. + virtual void endingShutdown() {} + /// Called after stopping the main loop and before releasing memory. /// Meant for quick/basic cleanup that does not require any other modules. virtual ~RegisteredRunner() {} @@ -82,6 +88,9 @@ public: /// registers a given runner with the given registry and returns registry count int RegisterRunner(RegisteredRunner *rr); +/// de-registers a given runner with the given registry and returns registry count +int DeregisterRunner(RegisteredRunner *rr); + /// Calls a given method of all runners. /// All runners are destroyed after the finishShutdown() call. void RunRegistered(const RegisteredRunner::Method &m); diff --git a/src/client_side.cc b/src/client_side.cc index 22abf07d55..87c7ad31a1 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -809,6 +809,7 @@ ConnStateData::swanSong() { debugs(33, 2, HERE << clientConnection); flags.readMore = false; + DeregisterRunner(this); clientdbEstablished(clientConnection->remote, -1); /* decrement */ assert(areAllContextsForThisConnection()); freeAllContexts(); @@ -1879,6 +1880,32 @@ ConnStateData::abortRequestParsing(const char *const uri) return context; } +void +ConnStateData::startShutdown() +{ + // RegisteredRunner API callback - Squid has been shut down + + // if connection is idle terminate it now, + // otherwise wait for grace period to end + if (getConcurrentRequestCount() == 0) + endingShutdown(); +} + +void +ConnStateData::endingShutdown() +{ + // RegisteredRunner API callback - Squid shutdown grace period is over + + // force the client connection to close immediately + // swanSong() in the close handler will cleanup. + if (Comm::IsConnOpen(clientConnection)) + clientConnection->close(); + + // deregister now to ensure finalShutdown() does not kill us prematurely. + // fd_table purge will cleanup if close handler was not fast enough. + DeregisterRunner(this); +} + char * skipLeadingSpace(char *aString) { @@ -3392,6 +3419,10 @@ ConnStateData::ConnStateData(const MasterXaction::Pointer &xact) : transferProtocol = port->transport; // default to the *_port protocol= setting. may change later. log_addr = xact->tcpClient->remote; log_addr.applyMask(Config.Addrs.client_netmask); + + // register to receive notice of Squid signal events + // which may affect long persisting client connections + RegisterRunner(this); } void diff --git a/src/client_side.h b/src/client_side.h index 5fedb74963..8657369743 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -11,6 +11,7 @@ #ifndef SQUID_CLIENTSIDE_H #define SQUID_CLIENTSIDE_H +#include "base/RunnersRegistry.h" #include "clientStreamForward.h" #include "comm.h" #include "helper/forward.h" @@ -167,7 +168,7 @@ class ServerBump; * * If the above can be confirmed accurate we can call this object PipelineManager or similar */ -class ConnStateData : public BodyProducer, public HttpControlMsgSink +class ConnStateData : public BodyProducer, public HttpControlMsgSink, public RegisteredRunner { public: @@ -418,6 +419,11 @@ public: /// client data which may need to forward as-is to server after an /// on_unsupported_protocol tunnel decision. SBuf preservedClientData; + + /* Registered Runner API */ + virtual void startShutdown(); + virtual void endingShutdown(); + protected: void startDechunkingRequest(); void finishDechunkingRequest(bool withSuccess); diff --git a/src/main.cc b/src/main.cc index 67459bb47a..ccf9dd4d82 100644 --- a/src/main.cc +++ b/src/main.cc @@ -211,6 +211,18 @@ private: EventLoop::Running->stop(); } + static void FinalShutdownRunners(void *) { + RunRegisteredHere(RegisteredRunner::endingShutdown); + + // XXX: this should be a Runner. +#if USE_AUTH + /* detach the auth components (only do this on full shutdown) */ + Auth::Scheme::FreeAll(); +#endif + + eventAdd("SquidTerminate", &StopEventLoop, NULL, 0, 1, false); + } + void doShutdown(time_t wait); void handleStoppedChild(); @@ -269,10 +281,6 @@ SignalEngine::doShutdown(time_t wait) /* run the closure code which can be shared with reconfigure */ serverConnectionsClose(); -#if USE_AUTH - /* detach the auth components (only do this on full shutdown) */ - Auth::Scheme::FreeAll(); -#endif RunRegisteredHere(RegisteredRunner::startShutdown); } @@ -281,7 +289,7 @@ SignalEngine::doShutdown(time_t wait) WIN32_svcstatusupdate(SERVICE_STOP_PENDING, (wait + 1) * 1000); #endif - eventAdd("SquidShutdown", &StopEventLoop, this, (double) (wait + 1), 1, false); + eventAdd("SquidShutdown", &FinalShutdownRunners, this, (double) (wait + 1), 1, false); } void