diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-20 12:52:40 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-20 12:52:40 +0000 |
commit | c7748cd34d9316e444a6005d2eba863ffbeac3cb (patch) | |
tree | 86783ed3cbc787ff5acb3269a61be3d3d350518a /config | |
parent | 199e224466a88fbe88bec7b897fe61dbabf9c5f5 (diff) |
- Remove partial thread safety with atomics.
- Add full thread safey with a lock.
Diffstat (limited to 'config')
-rw-r--r-- | config/src/tests/frtconnectionpool/frtconnectionpool.cpp | 5 | ||||
-rw-r--r-- | config/src/vespa/config/frt/frtconnection.cpp | 23 | ||||
-rw-r--r-- | config/src/vespa/config/frt/frtconnection.h | 15 |
3 files changed, 17 insertions, 26 deletions
diff --git a/config/src/tests/frtconnectionpool/frtconnectionpool.cpp b/config/src/tests/frtconnectionpool/frtconnectionpool.cpp index 7b3a22893ab..9c3498a92a1 100644 --- a/config/src/tests/frtconnectionpool/frtconnectionpool.cpp +++ b/config/src/tests/frtconnectionpool/frtconnectionpool.cpp @@ -217,9 +217,10 @@ void Test::testSetErrorAllHashBased() { */ void Test::testSuspensionTimeout() { const ServerSpec spec(_sources); - FRTConnectionPool sourcePool(_transport, spec, timingValues); + TimingValues short_transient_delay; + short_transient_delay.transientDelay = 1s; + FRTConnectionPool sourcePool(_transport, spec, short_transient_delay); FRTConnection* source = dynamic_cast<FRTConnection *>(sourcePool.getCurrent()); - source->setTransientDelay(1s); source->setError(FRTE_RPC_CONNECTION); for (int i = 0; i < 9; i++) { EXPECT_NOT_EQUAL(source->getAddress(), sourcePool.getCurrent()->getAddress()); diff --git a/config/src/vespa/config/frt/frtconnection.cpp b/config/src/vespa/config/frt/frtconnection.cpp index 31b2f25dae0..0c5d21ae4e9 100644 --- a/config/src/vespa/config/frt/frtconnection.cpp +++ b/config/src/vespa/config/frt/frtconnection.cpp @@ -14,19 +14,21 @@ namespace config { FRTConnection::FRTConnection(const vespalib::string& address, FRT_Supervisor& supervisor, const TimingValues & timingValues) : _address(address), + _transientDelay(timingValues.transientDelay), _fatalDelay(timingValues.fatalDelay), _supervisor(supervisor), + _lock(), _target(0), _suspendedUntil(), _suspendWarned(), _transientFailures(0), - _fatalFailures(0), - _transientDelay(timingValues.transientDelay) + _fatalFailures(0) { } FRTConnection::~FRTConnection() { + std::lock_guard guard(_lock); if (_target != nullptr) { LOG(debug, "Shutting down %s", _address.c_str()); _target->SubRef(); @@ -37,6 +39,7 @@ FRTConnection::~FRTConnection() FRT_Target * FRTConnection::getTarget() { + std::lock_guard guard(_lock); if (_target == nullptr) { _target = _supervisor.GetTarget(_address.c_str()); } else if ( ! _target->IsValid()) { @@ -78,6 +81,7 @@ FRTConnection::setError(int errorCode) void FRTConnection::setSuccess() { + std::lock_guard guard(_lock); _transientFailures = 0; _fatalFailures = 0; _suspendedUntil = steady_time(); @@ -86,24 +90,17 @@ void FRTConnection::setSuccess() void FRTConnection::calculateSuspension(ErrorType type) { duration delay = duration::zero(); + steady_time now = steady_clock::now(); + std::lock_guard guard(_lock); switch(type) { case TRANSIENT: - _transientFailures.fetch_add(1); - delay = _transientFailures.load(std::memory_order_relaxed) * getTransientDelay(); - if (delay > getMaxTransientDelay()) { - delay = getMaxTransientDelay(); - } + delay = std::min(6u, ++_transientFailures) * _transientDelay; LOG(warning, "Connection to %s failed or timed out", _address.c_str()); break; case FATAL: - _fatalFailures.fetch_add(1); - delay = _fatalFailures.load(std::memory_order_relaxed) * getFatalDelay(); - if (delay > getMaxFatalDelay()) { - delay = getMaxFatalDelay(); - } + delay = std::min(6u, ++_fatalFailures) * _fatalDelay; break; } - steady_time now = steady_clock::now(); _suspendedUntil = now + delay; if (_suspendWarned < (now - 5s)) { LOG(warning, "FRT Connection %s suspended until %s", _address.c_str(), vespalib::to_string(to_utc(_suspendedUntil)).c_str()); diff --git a/config/src/vespa/config/frt/frtconnection.h b/config/src/vespa/config/frt/frtconnection.h index 24dc9d657e0..a85c29863f0 100644 --- a/config/src/vespa/config/frt/frtconnection.h +++ b/config/src/vespa/config/frt/frtconnection.h @@ -3,7 +3,6 @@ #include "connection.h" #include <vespa/config/common/timingvalues.h> -#include <atomic> #include <memory> class FRT_Supervisor; @@ -27,27 +26,21 @@ public: vespalib::steady_time getSuspendedUntil() const { return _suspendedUntil; } void setError(int errorCode) override; void setSuccess(); - - /// Only used for testing - void setTransientDelay(duration delay) { _transientDelay = delay; } private: FRT_Target * getTarget(); void calculateSuspension(ErrorType type); - duration getTransientDelay() const { return _transientDelay; } - duration getMaxTransientDelay() const { return getTransientDelay() * 6; } - duration getMaxFatalDelay() const { return getFatalDelay() * 6; } - duration getFatalDelay() const { return _fatalDelay; } const vespalib::string _address; + const duration _transientDelay; const duration _fatalDelay; FRT_Supervisor& _supervisor; + std::mutex _lock; FRT_Target* _target; vespalib::steady_time _suspendedUntil; vespalib::steady_time _suspendWarned; - std::atomic<int> _transientFailures; - std::atomic<int> _fatalFailures; - duration _transientDelay; + uint32_t _transientFailures; + uint32_t _fatalFailures; }; } // namespace config |