Make StorageNode stop-request flag atomic

Also change requestShutdown() to be safe in the face of concurrent calls.

2 files changed, 10 insertions, 5 deletions

diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index 34db3b102e7..588ce356503 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -375,7 +375,7 @@ StorageNode::shutdown()
     // we might be shutting down after init exception causing only parts
     // of the server to have initialize
     LOG(debug, "Shutting down storage node of type %s", getNodeType().toString().c_str());
-    if (!_attemptedStopped) {
+    if (!attemptedStopped()) {
         LOG(debug, "Storage killed before requestShutdown() was called. No "
                    "reason has been given for why we're stopping.");
     }
@@ -518,7 +518,7 @@ void StorageNode::configure(std::unique_ptr<BucketspacesConfig> config) {
 bool
 StorageNode::attemptedStopped() const
 {
-    return _attemptedStopped;
+    return _attemptedStopped.load(std::memory_order_relaxed);
 }
 
 void
@@ -550,7 +550,12 @@ StorageNode::waitUntilInitialized(uint32_t timeout) {
 void
 StorageNode::requestShutdown(vespalib::stringref reason)
 {
-    if (_attemptedStopped) return;
+    bool was_stopped = false;
+    const bool stop_now = _attemptedStopped.compare_exchange_strong(was_stopped, true,
+                                                                    std::memory_order_relaxed, std::memory_order_relaxed);
+    if (!stop_now) {
+        return; // Someone else beat us to it.
+    }
     if (_component) {
         NodeStateUpdater::Lock::SP lock(_component->getStateUpdater().grabStateChangeLock());
         lib::NodeState nodeState(*_component->getStateUpdater().getReportedNodeState());
@@ -560,7 +565,6 @@ StorageNode::requestShutdown(vespalib::stringref reason)
             _component->getStateUpdater().setReportedNodeState(nodeState);
         }
     }
-    _attemptedStopped = true;
 }
 
 std::unique_ptr<StateManager>
diff --git a/storage/src/vespa/storage/storageserver/storagenode.h b/storage/src/vespa/storage/storageserver/storagenode.h
index 2e13c0e24c0..1778d98462c 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.h
+++ b/storage/src/vespa/storage/storageserver/storagenode.h
@@ -23,6 +23,7 @@
 #include <vespa/storage/storageutil/resumeguard.h>
 #include <vespa/storageframework/defaultimplementation/component/componentregisterimpl.h>
 #include <vespa/storageframework/generic/metric/metricupdatehook.h>
+#include <atomic>
 #include <mutex>
 
 namespace document { class DocumentTypeRepo; }
@@ -109,7 +110,7 @@ private:
     StorageNodeContext& _context;
     ApplicationGenerationFetcher& _generationFetcher;
     vespalib::string _rootFolder;
-    bool _attemptedStopped;
+    std::atomic<bool> _attemptedStopped;
     vespalib::string _pidFile;
 
     // First components that doesn't depend on others