diff --git a/CHANGELOG b/CHANGELOG index 1e04cbd16d..2c2537c551 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,9 +1,6 @@ v3.4.9 (XXXX-XX-XX) ------------------- -* The keep alive timeout specified via --http.keep-alive-timeout is now being - honored. - * Harden database creation against spurious "duplicate name" errors that were caused by other parallel operations lazily creating required system collections in the same database. @@ -152,8 +149,8 @@ v3.4.8 (2019-09-09) * Prevent rare cases of duplicate DDL actions being executed by Maintenance. -* Fixed coordinator code that was reporting rocksdb error codes, but not the associated detail - message. +* Coordinator code was reporting rocksdb error codes, but not the associated detail message. + Corrected. * Fixed some error reporting and logging in Maintenance. diff --git a/arangod/GeneralServer/HttpCommTask.cpp b/arangod/GeneralServer/HttpCommTask.cpp index 46cd38730e..6d7b878b36 100644 --- a/arangod/GeneralServer/HttpCommTask.cpp +++ b/arangod/GeneralServer/HttpCommTask.cpp @@ -108,11 +108,10 @@ void HttpCommTask::addResponse(GeneralResponse& baseResponse, RequestStatistics* #endif finishExecution(baseResponse); + resetKeepAlive(); // response has been queued, allow further requests _requestPending = false; - - resetKeepAlive(); // CORS response handling if (!_origin.empty()) { @@ -220,14 +219,12 @@ void HttpCommTask::addResponse(GeneralResponse& baseResponse, RequestStatistics* bool HttpCommTask::processRead(double startTime) { TRI_ASSERT(_peer->runningInThisThread()); + cancelKeepAlive(); TRI_ASSERT(_readBuffer.c_str() != nullptr); if (_requestPending) { return false; } - - // starts and extends the keep-alive timeout - resetKeepAlive(); // will extend the Keep-Alive timeout RequestStatistics* stat = nullptr; bool handleRequest = false; @@ -600,7 +597,6 @@ bool HttpCommTask::processRead(double startTime) { void HttpCommTask::processRequest(std::unique_ptr request) { TRI_ASSERT(_peer->runningInThisThread()); - cancelKeepAlive(); // timeout will be restarted { LOG_TOPIC(DEBUG, Logger::REQUESTS) diff --git a/arangod/Scheduler/SocketTask.cpp b/arangod/Scheduler/SocketTask.cpp index 5c85779faf..1e9d026798 100644 --- a/arangod/Scheduler/SocketTask.cpp +++ b/arangod/Scheduler/SocketTask.cpp @@ -230,36 +230,32 @@ void SocketTask::addToReadBuffer(char const* data, std::size_t len) { // does not need lock void SocketTask::resetKeepAlive() { - if (!_useKeepAliveTimer) { - return; - } - - // expires_from_now cancels pending operations - asio_ns::error_code err; - _keepAliveTimer->expires_from_now(_keepAliveTimeout, err); - if (err) { - closeStream(); - return; - } - _keepAliveTimerActive.store(true); - - std::weak_ptr self = shared_from_this(); - _keepAliveTimer->async_wait([self, this](const asio_ns::error_code& ec) { - if (!ec) { // error will be true if timer was canceled - LOG_TOPIC(INFO, Logger::COMMUNICATION) - << "keep alive timout - closing stream!"; - if (auto s = self.lock()) { - this->closeStream(); - } + if (_useKeepAliveTimer) { + asio_ns::error_code err; + _keepAliveTimer->expires_from_now(_keepAliveTimeout, err); + if (err) { + closeStream(); + return; } - }); + + _keepAliveTimerActive.store(true, std::memory_order_relaxed); + auto self = shared_from_this(); + _keepAliveTimer->async_wait([self, this](const asio_ns::error_code& error) { + if (!error) { // error will be true if timer was canceled + LOG_TOPIC(ERR, Logger::COMMUNICATION) + << "keep alive timout - closing stream!"; + closeStream(); + } + }); + } } // caller must hold the _lock void SocketTask::cancelKeepAlive() { - if (_keepAliveTimerActive.exchange(false)) { + if (_useKeepAliveTimer && _keepAliveTimerActive.load(std::memory_order_relaxed)) { asio_ns::error_code err; _keepAliveTimer->cancel(err); + _keepAliveTimerActive.store(false, std::memory_order_relaxed); } } diff --git a/arangod/Scheduler/SocketTask.h b/arangod/Scheduler/SocketTask.h index 070871d290..4b4244db6a 100644 --- a/arangod/Scheduler/SocketTask.h +++ b/arangod/Scheduler/SocketTask.h @@ -138,7 +138,7 @@ class SocketTask : virtual public Task { // caller must run in _peer->strand() void closeStreamNoLock(); - // starts the keep alive time + // starts the keep alive time, no need to run on strand void resetKeepAlive(); // cancels the keep alive timer