Use a trinary to also handle temporary mismatching bucket used bits.

3 files changed, 23 insertions, 14 deletions

diff --git a/persistence/src/tests/spi/clusterstatetest.cpp b/persistence/src/tests/spi/clusterstatetest.cpp
index d60330690a9..17c4c3dc635 100644
--- a/persistence/src/tests/spi/clusterstatetest.cpp
+++ b/persistence/src/tests/spi/clusterstatetest.cpp
@@ -8,6 +8,7 @@
 #include <gtest/gtest.h>
 
 using storage::spi::test::makeSpiBucket;
+using vespalib::Trinary;
 
 namespace storage::spi {
 
@@ -154,35 +155,35 @@ TEST(ClusterStateTest, testReady)
     {
         lib::Distribution d(getCfg(3, 0));
         ClusterState state(s, 0, d);
-        EXPECT_EQ(false, state.shouldBeReady(b));
+        EXPECT_EQ(Trinary::False, state.shouldBeReady(b));
     }
 
     // Only node 0 with 1 ready copy.
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 1));
         ClusterState state(s, i, d);
-        EXPECT_EQ(i == 0, state.shouldBeReady(b));
+        EXPECT_EQ((i == 0) ? Trinary::True : Trinary::False, state.shouldBeReady(b));
     }
 
     // All of them with 3 ready copies
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 3));
         ClusterState state(s, i, d);
-        EXPECT_EQ(true, state.shouldBeReady(b));
+        EXPECT_EQ(Trinary::True, state.shouldBeReady(b));
     }
 
     // Node 0 and node 1 with 2 ready copies.
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 2));
         ClusterState state(s, i, d);
-        EXPECT_EQ(i == 0 || i == 2, state.shouldBeReady(b));
+        EXPECT_EQ((i == 0 || i == 2) ? Trinary::True : Trinary::False, state.shouldBeReady(b));
     }
 
     // All of them with 3 ready copies
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 3));
         ClusterState state(s, i, d);
-        EXPECT_EQ(true, state.shouldBeReady(b));
+        EXPECT_EQ(Trinary::True, state.shouldBeReady(b));
     }
 
     lib::ClusterState s2("version:1 storage:3 .0.s:d distributor:3");
@@ -191,13 +192,13 @@ TEST(ClusterStateTest, testReady)
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 2));
         ClusterState state(s2, i, d);
-        EXPECT_EQ(i == 1 || i == 2, state.shouldBeReady(b));
+        EXPECT_EQ((i == 1 || i == 2) ? Trinary::True : Trinary::False, state.shouldBeReady(b));
     }
 
     for (uint32_t i = 0; i < 3; ++i) {
         lib::Distribution d(getCfg(3, 1));
         ClusterState state(s2, i, d);
-        EXPECT_EQ(i == 2, state.shouldBeReady(b));
+        EXPECT_EQ((i == 2) ? Trinary::True : Trinary::False, state.shouldBeReady(b));
     }
 }
 
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.cpp b/persistence/src/vespa/persistence/spi/clusterstate.cpp
index d3c7afda222..03e3d3ee02f 100644
--- a/persistence/src/vespa/persistence/spi/clusterstate.cpp
+++ b/persistence/src/vespa/persistence/spi/clusterstate.cpp
@@ -8,6 +8,8 @@
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <cassert>
 
+using vespalib::Trinary;
+
 namespace storage::spi {
 
 ClusterState::ClusterState(const lib::ClusterState& state,
@@ -43,12 +45,17 @@ ClusterState::ClusterState(const ClusterState& other) {
 
 ClusterState::~ClusterState() = default;
 
-bool ClusterState::shouldBeReady(const Bucket& b) const {
+Trinary
+ClusterState::shouldBeReady(const Bucket& b) const {
     assert(_distribution);
     assert(_state);
 
+    if (b.getBucketId().getUsedBits() < _state->getDistributionBitCount()) {
+        return Trinary::Undefined;
+    }
+
     if (_distribution->getReadyCopies() >= _distribution->getRedundancy()) {
-        return true; // all copies should be ready
+        return Trinary::True; // all copies should be ready
     }
 
     std::vector<uint16_t> idealNodes;
@@ -56,9 +63,9 @@ bool ClusterState::shouldBeReady(const Bucket& b) const {
                                  b.getBucketId(), idealNodes,
                                  "uim", _distribution->getReadyCopies());
     for (uint32_t i=0, n=idealNodes.size(); i<n; ++i) {
-        if (idealNodes[i] == _nodeIndex) return true;
+        if (idealNodes[i] == _nodeIndex) return Trinary::True;
     }
-    return false;
+    return Trinary::False;
 }
 
 bool ClusterState::clusterUp() const {
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.h b/persistence/src/vespa/persistence/spi/clusterstate.h
index 5ae692f7661..1333c44ec51 100644
--- a/persistence/src/vespa/persistence/spi/clusterstate.h
+++ b/persistence/src/vespa/persistence/spi/clusterstate.h
@@ -2,6 +2,7 @@
 #pragma once
 
 #include <memory>
+#include <vespa/vespalib/util/trinary.h>
 
 namespace vespalib { class nbostream; }
 namespace storage::lib {
@@ -30,13 +31,13 @@ public:
     ~ClusterState();
 
     /**
-     * Returns true if the system has been set up to have
+     * Returns Trinary::True if the system has been set up to have
      * "ready" nodes, and the given bucket is in the ideal state
-     * for readiness.
+     * for readiness. Trinary ::Undefined is returned in case the bucketId is invalid (too few used bits)
      *
      * @param b The bucket to check.
      */
-    bool shouldBeReady(const Bucket& b) const;
+    vespalib::Trinary shouldBeReady(const Bucket& b) const;
 
     /**
      * Returns false if the cluster has been deemed down. This can happen