make soft doom factor atomic

also avoid actual value race by not assigning it to the initial value
for a (very) short time each time stats are sampled.

3 files changed, 16 insertions, 13 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
index 756af216988..381915f53d4 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp
@@ -128,8 +128,7 @@ Matcher::getStats()
 {
     std::lock_guard<std::mutex> guard(_statsLock);
     MatchingStats stats = std::move(_stats);
-    _stats = MatchingStats();
-    _stats.softDoomFactor(stats.softDoomFactor());
+    _stats = MatchingStats(stats.softDoomFactor());
     return stats;
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp
index 2c826094ddf..bbb9dba9a30 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.cpp
@@ -19,7 +19,7 @@ constexpr double MAX_CHANGE_FACTOR = 5;
 
 } // namespace proton::matching::<unnamed>
 
-MatchingStats::MatchingStats()
+MatchingStats::MatchingStats(double prev_soft_doom_factor)
     : _queries(0),
       _limited_queries(0),
       _docidSpaceCovered(0),
@@ -28,7 +28,7 @@ MatchingStats::MatchingStats()
       _docsReRanked(0),
       _softDoomed(0),
       _doomOvertime(),
-      _softDoomFactor(INITIAL_SOFT_DOOM_FACTOR),
+      _softDoomFactor(prev_soft_doom_factor),
       _queryCollateralTime(), // TODO: Remove in Vespa 8
       _querySetupTime(),
       _queryLatency(),
@@ -89,16 +89,18 @@ MatchingStats::updatesoftDoomFactor(vespalib::duration hardLimit, vespalib::dura
     // It is merely a safety measure to avoid overflow on bad input as can happen with time senstive stuff
     // in any soft real time system.
     if ((hardLimit >= MIN_TIMEOUT) && (softLimit >= MIN_TIMEOUT)) {
+        double factor = softDoomFactor();
         double diff = vespalib::to_s(softLimit - duration)/vespalib::to_s(hardLimit);
         if (duration < softLimit) {
             // Since softdoom factor can become very small, allow a minimum change of some size
-            diff = std::min(diff, _softDoomFactor*MAX_CHANGE_FACTOR);
-            _softDoomFactor += 0.01*diff;
+            diff = std::min(diff, factor*MAX_CHANGE_FACTOR);
+            factor += 0.01*diff;
         } else {
-            diff = std::max(diff, -_softDoomFactor*MAX_CHANGE_FACTOR);
-            _softDoomFactor += 0.02*diff;
+            diff = std::max(diff, -factor*MAX_CHANGE_FACTOR);
+            factor += 0.02*diff;
         }
-        _softDoomFactor = std::max(_softDoomFactor, 0.01); // Never go below 1%
+        factor = std::max(factor, 0.01); // Never go below 1%
+        softDoomFactor(factor);
     }
     return *this;
 }
diff --git a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h
index 047c6fcaf13..eafa14870fa 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h
+++ b/searchcore/src/vespa/searchcore/proton/matching/matching_stats.h
@@ -5,6 +5,7 @@
 #include <vector>
 #include <cstddef>
 #include <vespa/vespalib/util/time.h>
+#include <vespa/vespalib/datastore/atomic_value_wrapper.h>
 
 namespace proton::matching {
 
@@ -124,7 +125,8 @@ private:
     size_t                 _docsReRanked;
     size_t                 _softDoomed;
     Avg                    _doomOvertime;
-    double                 _softDoomFactor;
+    using SoftDoomFactor = vespalib::datastore::AtomicValueWrapper<double>;
+    SoftDoomFactor         _softDoomFactor;
     Avg                    _queryCollateralTime; // TODO: Remove in Vespa 8
     Avg                    _querySetupTime;
     Avg                    _queryLatency;
@@ -139,7 +141,7 @@ public:
     MatchingStats & operator = (const MatchingStats &) = delete;
     MatchingStats(MatchingStats &&) = default;
     MatchingStats & operator =  (MatchingStats &&) = default;
-    MatchingStats();
+    MatchingStats(double prev_soft_doom_factor = INITIAL_SOFT_DOOM_FACTOR);
     ~MatchingStats();
 
     MatchingStats &queries(size_t value) { _queries = value; return *this; }
@@ -165,8 +167,8 @@ public:
 
     vespalib::duration doomOvertime() const { return vespalib::from_s(_doomOvertime.max()); }
 
-    MatchingStats &softDoomFactor(double value) { _softDoomFactor = value; return *this; }
-    double softDoomFactor() const { return _softDoomFactor; }
+    MatchingStats &softDoomFactor(double value) { _softDoomFactor.store_relaxed(value); return *this; }
+    double softDoomFactor() const { return _softDoomFactor.load_relaxed(); }
     MatchingStats &updatesoftDoomFactor(vespalib::duration hardLimit, vespalib::duration softLimit, vespalib::duration duration);
 
     // TODO: Remove in Vespa 8