Merge pull request #20156 from vespa-engine/vekterli/allow-searches-when-node-is-in-maintenance-mode

Continue serving search queries when in Maintenance node state [run-systemtest]

3 files changed, 58 insertions, 18 deletions

diff --git a/persistence/src/tests/spi/clusterstatetest.cpp b/persistence/src/tests/spi/clusterstatetest.cpp
index 8a303c1a1ac..ac67903244f 100644
--- a/persistence/src/tests/spi/clusterstatetest.cpp
+++ b/persistence/src/tests/spi/clusterstatetest.cpp
@@ -233,4 +233,31 @@ TEST(ClusterStateTest, can_infer_own_node_retired_state)
     EXPECT_TRUE(!node_marked_as_retired_in_state("distributor:3 storage:3 .1.s:r", d, 0));
 }
 
+namespace {
+
+bool
+node_marked_as_maintenance_in_state(const std::string& stateStr,
+                                    const lib::Distribution& d,
+                                    uint16_t node,
+                                    bool maintenance_in_all_spaces)
+{
+    lib::ClusterState s(stateStr);
+    ClusterState state(s, node, d, maintenance_in_all_spaces);
+    return state.nodeMaintenance();
+}
+
+}
+
+// We want to track the maintenance state for the _node_, not just the _bucket space_.
+TEST(ClusterStateTest, node_maintenance_state_is_set_independent_of_bucket_space_state_string)
+{
+    lib::Distribution d(lib::Distribution::getDefaultDistributionConfig(3, 3));
+
+    // Note: it doesn't actually matter what the cluster state string itself says here
+    EXPECT_FALSE(node_marked_as_maintenance_in_state("distributor:3 storage:3", d, 0, false));
+    EXPECT_TRUE(node_marked_as_maintenance_in_state("distributor:3 storage:3", d, 0, true));
+    EXPECT_TRUE(node_marked_as_maintenance_in_state("distributor:3 storage:3 .0.s:d", d, 0, true));
+    EXPECT_FALSE(node_marked_as_maintenance_in_state("distributor:3 storage:3 .0.s:m", d, 0, false));
+}
+
 }
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.cpp b/persistence/src/vespa/persistence/spi/clusterstate.cpp
index 4bc538996ca..f82e6165fb8 100644
--- a/persistence/src/vespa/persistence/spi/clusterstate.cpp
+++ b/persistence/src/vespa/persistence/spi/clusterstate.cpp
@@ -14,10 +14,12 @@ namespace storage::spi {
 
 ClusterState::ClusterState(const lib::ClusterState& state,
                            uint16_t nodeIndex,
-                           const lib::Distribution& distribution)
+                           const lib::Distribution& distribution,
+                           bool maintenanceInAllSpaces)
     : _state(std::make_unique<lib::ClusterState>(state)),
       _distribution(std::make_unique<lib::Distribution>(distribution.serialize())),
-      _nodeIndex(nodeIndex)
+      _nodeIndex(nodeIndex),
+      _maintenanceInAllSpaces(maintenanceInAllSpaces)
 {
 }
 
@@ -33,14 +35,11 @@ void ClusterState::deserialize(vespalib::nbostream& i) {
     _distribution = std::make_unique<lib::Distribution>(distribution);
 }
 
-ClusterState::ClusterState(vespalib::nbostream& i) {
-    deserialize(i);
-}
-
 ClusterState::ClusterState(const ClusterState& other) {
     vespalib::nbostream o;
     other.serialize(o);
     deserialize(o);
+    _maintenanceInAllSpaces = other._maintenanceInAllSpaces;
 }
 
 ClusterState::~ClusterState() = default;
@@ -68,28 +67,32 @@ ClusterState::shouldBeReady(const Bucket& b) const {
     return Trinary::False;
 }
 
-bool ClusterState::clusterUp() const {
+bool ClusterState::clusterUp() const noexcept {
     return _state && _state->getClusterState() == lib::State::UP;
 }
 
-bool ClusterState::nodeHasStateOneOf(const char* states) const {
+bool ClusterState::nodeHasStateOneOf(const char* states) const noexcept {
     return _state &&
            _state->getNodeState(lib::Node(lib::NodeType::STORAGE, _nodeIndex)).
                    getState().oneOf(states);
 }
 
-bool ClusterState::nodeUp() const {
+bool ClusterState::nodeUp() const noexcept {
     return nodeHasStateOneOf("uir");
 }
 
-bool ClusterState::nodeInitializing() const {
+bool ClusterState::nodeInitializing() const noexcept {
     return nodeHasStateOneOf("i");
 }
 
-bool ClusterState::nodeRetired() const {
+bool ClusterState::nodeRetired() const noexcept {
     return nodeHasStateOneOf("r");
 }
 
+bool ClusterState::nodeMaintenance() const noexcept {
+    return _maintenanceInAllSpaces;
+}
+
 void ClusterState::serialize(vespalib::nbostream& o) const {
     assert(_distribution);
     assert(_state);
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.h b/persistence/src/vespa/persistence/spi/clusterstate.h
index 8e48758e243..bde7e5bdbf4 100644
--- a/persistence/src/vespa/persistence/spi/clusterstate.h
+++ b/persistence/src/vespa/persistence/spi/clusterstate.h
@@ -23,9 +23,9 @@ public:
 
     ClusterState(const lib::ClusterState& state,
                  uint16_t nodeIndex,
-                 const lib::Distribution& distribution);
+                 const lib::Distribution& distribution,
+                 bool maintenanceInAllSpaces = false);
 
-    ClusterState(vespalib::nbostream& i);
     ClusterState(const ClusterState& other);
     ClusterState& operator=(const ClusterState& other) = delete;
     ~ClusterState();
@@ -45,23 +45,32 @@ public:
      * compared to the complete list of nodes, and deigns the system to be
      * unusable.
      */
-    bool clusterUp() const;
+    [[nodiscard]] bool clusterUp() const noexcept;
 
     /**
      * Returns false if this node has been set in a state where it should not
      * receive external load.
+     *
+     * TODO rename to indicate bucket space affinity.
      */
-    bool nodeUp() const;
+    [[nodiscard]] bool nodeUp() const noexcept;
 
     /**
      * Returns true iff this node is marked as Initializing in the cluster state.
+     *
+     * TODO remove, init no longer used internally.
      */
-    bool nodeInitializing() const;
+    [[nodiscard]] bool nodeInitializing() const noexcept;
 
     /**
      * Returns true iff this node is marked as Retired in the cluster state.
      */
-    bool nodeRetired() const;
+    [[nodiscard]] bool nodeRetired() const noexcept;
+
+    /**
+     * Returns true iff this node is marked as Maintenance in all bucket space cluster states.
+     */
+    [[nodiscard]] bool nodeMaintenance() const noexcept;
 
     /**
      * Returns a serialized form of this object.
@@ -72,9 +81,10 @@ private:
     std::unique_ptr<lib::ClusterState> _state;
     std::unique_ptr<lib::Distribution> _distribution;
     uint16_t _nodeIndex;
+    bool _maintenanceInAllSpaces;
 
     void deserialize(vespalib::nbostream&);
-    bool nodeHasStateOneOf(const char* states) const;
+    bool nodeHasStateOneOf(const char* states) const noexcept;
 };
 
 }