From 9633e7eab41bde2dba7083af4846b754e661fd0c Mon Sep 17 00:00:00 2001
From: Tor Brede Vekterli <vekterli@vespa.ai>
Date: Wed, 25 Oct 2023 13:30:22 +0000
Subject: Print Bouncer state within lock to ensure visibility

This code path is only encountered when debug logging is explicitly
enabled for the parent `StorageLink` component. Turns out an old
system test did just that.
---
 storage/src/vespa/storage/storageserver/bouncer.cpp |  5 +++--
 storage/src/vespa/storage/storageserver/bouncer.h   | 16 ++++++++--------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/storage/src/vespa/storage/storageserver/bouncer.cpp b/storage/src/vespa/storage/storageserver/bouncer.cpp
index 7e3b21ef33a..bfc38e0c8ba 100644
--- a/storage/src/vespa/storage/storageserver/bouncer.cpp
+++ b/storage/src/vespa/storage/storageserver/bouncer.cpp
@@ -47,6 +47,7 @@ Bouncer::print(std::ostream& out, bool verbose,
                const std::string& indent) const
 {
     (void) verbose; (void) indent;
+    std::lock_guard guard(_lock);
     out << "Bouncer(" << _baselineNodeState << ")";
 }
 
@@ -343,9 +344,9 @@ void
 Bouncer::handleNewState() noexcept
 {
     std::lock_guard lock(_lock);
-    const auto reportedNodeState = *_component.getStateUpdater().getReportedNodeState();
+    const auto reportedNodeState  = *_component.getStateUpdater().getReportedNodeState();
     const auto clusterStateBundle = _component.getStateUpdater().getClusterStateBundle();
-    const auto &clusterState = *clusterStateBundle->getBaselineClusterState();
+    const auto& clusterState      = *clusterStateBundle->getBaselineClusterState();
     _clusterState = &clusterState.getClusterState();
     const lib::Node node(_component.getNodeType(), _component.getIndex());
     _baselineNodeState = deriveNodeState(reportedNodeState, clusterState.getNodeState(node));
diff --git a/storage/src/vespa/storage/storageserver/bouncer.h b/storage/src/vespa/storage/storageserver/bouncer.h
index 44c7a16f3dc..26282625269 100644
--- a/storage/src/vespa/storage/storageserver/bouncer.h
+++ b/storage/src/vespa/storage/storageserver/bouncer.h
@@ -27,16 +27,16 @@ class Bouncer : public StorageLink,
                 private StateListener
 {
     using StorBouncerConfig = vespa::config::content::core::StorBouncerConfig;
+    using BucketSpaceNodeStateMapping = std::unordered_map<document::BucketSpace, lib::NodeState, document::BucketSpace::hash>;
 
     std::unique_ptr<StorBouncerConfig> _config;
-    StorageComponent _component;
-    std::mutex       _lock;
-    lib::NodeState   _baselineNodeState;
-    using BucketSpaceNodeStateMapping = std::unordered_map<document::BucketSpace, lib::NodeState, document::BucketSpace::hash>;
-    BucketSpaceNodeStateMapping _derivedNodeStates;
-    const lib::State* _clusterState;
-    std::unique_ptr<BouncerMetrics> _metrics;
-    bool _closed;
+    StorageComponent                   _component;
+    mutable std::mutex                 _lock;
+    lib::NodeState                     _baselineNodeState;
+    BucketSpaceNodeStateMapping        _derivedNodeStates;
+    const lib::State*                  _clusterState;
+    std::unique_ptr<BouncerMetrics>    _metrics;
+    bool                               _closed;
 
 public:
     Bouncer(StorageComponentRegister& compReg, const StorBouncerConfig& bootstrap_config);
-- 
cgit v1.2.3