- Remove partial thread safety with atomics.

- Add full thread safey with a lock.

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