Merge pull request #24492 from vespa-engine/balder/statecheckers-are-stateless

Use std::make_shared for StateCheckers.

6 files changed, 122 insertions, 202 deletions

diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp
index a2e97938a73..728040da50e 100644
--- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp
+++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp
@@ -4,15 +4,14 @@
 #include "statecheckers.h"
 #include "top_level_distributor.h"
 #include "idealstatemetricsset.h"
+#include "distributor_bucket_space_repo.h"
+#include "distributor_bucket_space.h"
 #include <vespa/vespalib/stllike/asciistream.h>
-#include <vespa/storage/storageserver/storagemetricsset.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <vespa/storage/common/bucketmessages.h>
 #include <vespa/document/bucket/fixed_bucket_spaces.h>
 #include <vespa/vespalib/util/assert.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
-#include "distributor_bucket_space_repo.h"
-#include "distributor_bucket_space.h"
 
 #include <vespa/log/log.h>
 LOG_SETUP(".distributor.operation.queue");
@@ -33,41 +32,29 @@ IdealStateManager::IdealStateManager(
       _has_logged_phantom_replica_warning(false)
 {
     LOG(debug, "Adding BucketStateStateChecker to state checkers");
-    _stateCheckers.push_back(StateChecker::SP(new BucketStateStateChecker()));
-
-    _splitBucketStateChecker = new SplitBucketStateChecker();
-    _stateCheckers.push_back(StateChecker::SP(_splitBucketStateChecker));
-    _stateCheckers.push_back(StateChecker::SP(new SplitInconsistentStateChecker()));
-    _stateCheckers.push_back(StateChecker::SP(new SynchronizeAndMoveStateChecker()));
-    _stateCheckers.push_back(StateChecker::SP(new JoinBucketsStateChecker()));
-    _stateCheckers.push_back(StateChecker::SP(new DeleteExtraCopiesStateChecker()));
-    _stateCheckers.push_back(StateChecker::SP(new GarbageCollectionStateChecker()));
+    _stateCheckers.emplace_back(std::make_shared<BucketStateStateChecker>());
+
+    _stateCheckers.emplace_back(std::make_shared<SplitBucketStateChecker>());
+    _splitBucketStateChecker = dynamic_cast<SplitBucketStateChecker *>(_stateCheckers.back().get());
+
+    _stateCheckers.emplace_back(std::make_shared<SplitInconsistentStateChecker>());
+    _stateCheckers.emplace_back(std::make_shared<SynchronizeAndMoveStateChecker>());
+    _stateCheckers.emplace_back(std::make_shared<JoinBucketsStateChecker>());
+    _stateCheckers.emplace_back(std::make_shared<DeleteExtraCopiesStateChecker>());
+    _stateCheckers.emplace_back(std::make_shared<GarbageCollectionStateChecker>());
 }
 
 IdealStateManager::~IdealStateManager() = default;
 
 void
-IdealStateManager::print(std::ostream& out, bool verbose,
-                         const std::string& indent) const
+IdealStateManager::print(std::ostream& out, bool verbose, const std::string& indent)
 {
     (void) verbose; (void) indent;
     out << "IdealStateManager";
 }
 
-bool
-IdealStateManager::iAmUp() const
-{
-    Node node(NodeType::DISTRIBUTOR, node_context().node_index());
-    // Assume that derived cluster states agree on distributor node being up
-    const auto &state = *operation_context().cluster_state_bundle().getBaselineClusterState();
-    const lib::State &nodeState = state.getNodeState(node).getState();
-    const lib::State &clusterState = state.getClusterState();
-
-    return (nodeState == lib::State::UP && clusterState == lib::State::UP);
-}
-
 void
-IdealStateManager::fillParentAndChildBuckets(StateChecker::Context& c) const
+IdealStateManager::fillParentAndChildBuckets(StateChecker::Context& c)
 {
     c.db.getAll(c.getBucketId(), c.entries);
     if (c.entries.empty()) {
@@ -77,21 +64,20 @@ IdealStateManager::fillParentAndChildBuckets(StateChecker::Context& c) const
     }
 }
 void
-IdealStateManager::fillSiblingBucket(StateChecker::Context& c) const
+IdealStateManager::fillSiblingBucket(StateChecker::Context& c)
 {
     c.siblingEntry = c.db.get(c.siblingBucket);
 }
 
 BucketDatabase::Entry*
-IdealStateManager::getEntryForPrimaryBucket(StateChecker::Context& c) const
+IdealStateManager::getEntryForPrimaryBucket(StateChecker::Context& c)
 {
-    for (uint32_t j = 0; j < c.entries.size(); ++j) {
-        BucketDatabase::Entry& e = c.entries[j];
+    for (auto & e : c.entries) {
         if (e.getBucketId() == c.getBucketId() && ! e->getNodes().empty()) {
             return &e;
         }
     }
-    return 0;
+    return nullptr;
 }
 
 namespace {
@@ -116,16 +102,15 @@ IdealStateManager::runStateCheckers(StateChecker::Context& c) const
     auto highestPri = StateChecker::Result::noMaintenanceNeeded();
     // We go through _all_ active state checkers so that statistics can be
     // collected across all checkers, not just the ones that are highest pri.
-    for (uint32_t i = 0; i < _stateCheckers.size(); i++) {
+    for (const auto & checker : _stateCheckers) {
         if (!operation_context().distributor_config().stateCheckerIsActive(
-                _stateCheckers[i]->getName()))
+                checker->getName()))
         {
-            LOG(spam, "Skipping state checker %s",
-                _stateCheckers[i]->getName());
+            LOG(spam, "Skipping state checker %s", checker->getName());
             continue;
         }
 
-        auto result = _stateCheckers[i]->check(c);
+        auto result = checker->check(c);
         if (canOverwriteResult(highestPri, result)) {
             highestPri = std::move(result);
         }
@@ -180,13 +165,12 @@ IdealStateManager::prioritize(
         const document::Bucket &bucket,
         NodeMaintenanceStatsTracker& statsTracker) const
 {
-    StateChecker::Result generated(
-            generateHighestPriority(bucket, statsTracker));
+    StateChecker::Result generated(generateHighestPriority(bucket, statsTracker));
     MaintenancePriority priority(generated.getPriority());
     MaintenanceOperation::Type type(priority.requiresMaintenance()
                                     ? generated.getType()
                                     : MaintenanceOperation::OPERATION_COUNT);
-    return MaintenancePriorityAndType(priority, type);
+    return {priority, type};
 }
 
 IdealStateOperation::SP
@@ -201,17 +185,16 @@ IdealStateManager::generateInterceptingSplit(BucketSpace bucketSpace,
     if (e.valid()) {
         c.entry = e;
 
-        IdealStateOperation::UP operation(
-                _splitBucketStateChecker->check(c).createOperation());
+        IdealStateOperation::UP operation(_splitBucketStateChecker->check(c).createOperation());
         if (operation.get()) {
             operation->setPriority(pri);
             operation->setIdealStateManager(this);
         }
 
-        return IdealStateOperation::SP(operation.release());
+        return operation;
     }
 
-    return IdealStateOperation::SP();
+    return {};
 }
 
 MaintenanceOperation::SP
@@ -243,11 +226,10 @@ IdealStateManager::generateAll(const document::Bucket &bucket,
         return operations;
     }
 
-    for (uint32_t i = 0; i < _stateCheckers.size(); i++) {
-        IdealStateOperation::UP op(
-                _stateCheckers[i]->check(c).createOperation());
-        if (op.get()) {
-            operations.push_back(IdealStateOperation::SP(op.release()));
+    for (const auto & checker : _stateCheckers) {
+        IdealStateOperation::UP op(checker->check(c).createOperation());
+        if (op) {
+            operations.emplace_back(std::move(op));
         }
     }
     return operations;
diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h
index 69f6b6dc670..32349541de2 100644
--- a/storage/src/vespa/storage/distributor/idealstatemanager.h
+++ b/storage/src/vespa/storage/distributor/idealstatemanager.h
@@ -5,7 +5,6 @@
 #include "statechecker.h"
 #include <vespa/storage/distributor/maintenance/maintenanceprioritygenerator.h>
 #include <vespa/storage/distributor/maintenance/maintenanceoperationgenerator.h>
-#include <vespa/storageframework/generic/status/htmlstatusreporter.h>
 
 namespace storage::distributor {
 
@@ -39,8 +38,7 @@ public:
 
     ~IdealStateManager() override;
 
-    void print(std::ostream& out, bool verbose,
-                       const std::string& indent) const;
+    static void print(std::ostream& out, bool verbose, const std::string& indent);
 
     // MaintenancePriorityGenerator interface
     MaintenancePriorityAndType prioritize(
@@ -79,14 +77,14 @@ public:
 
 private:
     void verify_only_live_nodes_in_context(const StateChecker::Context& c) const;
-    void fillParentAndChildBuckets(StateChecker::Context& c) const;
-    void fillSiblingBucket(StateChecker::Context& c) const;
+    static void fillParentAndChildBuckets(StateChecker::Context& c);
+    static void fillSiblingBucket(StateChecker::Context& c);
     StateChecker::Result generateHighestPriority(
             const document::Bucket &bucket,
             NodeMaintenanceStatsTracker& statsTracker) const;
     StateChecker::Result runStateCheckers(StateChecker::Context& c) const;
 
-    BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c) const;
+    static BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c);
 
     IdealStateMetricSet& _metrics;
     document::BucketId _lastPrioritizedBucket;
@@ -100,8 +98,6 @@ private:
     DistributorStripeOperationContext& _op_ctx;
     mutable bool _has_logged_phantom_replica_warning;
 
-    bool iAmUp() const;
-
     class StatusBucketVisitor : public BucketDatabase::EntryProcessor {
         // Stats tracker to use for all generateAll() calls to avoid having
         // to create a new hash map for each single bucket processed.
diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp
index e5a1822d455..91acd494a7a 100644
--- a/storage/src/vespa/storage/distributor/statechecker.cpp
+++ b/storage/src/vespa/storage/distributor/statechecker.cpp
@@ -103,7 +103,4 @@ StateChecker::Context::toString() const
     return ss.str();
 }
 
-StateChecker::StateChecker()  = default;
-StateChecker::~StateChecker() = default;
-
 }
diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h
index b72898208e6..11c83fdde07 100644
--- a/storage/src/vespa/storage/distributor/statechecker.h
+++ b/storage/src/vespa/storage/distributor/statechecker.h
@@ -109,19 +109,19 @@ public:
         std::unique_ptr<ResultImpl> _impl;
     public:
         IdealStateOperation::UP createOperation() {
-            return (_impl.get() 
+            return (_impl
                     ? _impl->createOperation()
                     : IdealStateOperation::UP());
         }
 
         MaintenancePriority getPriority() const {
-            return (_impl.get()
+            return (_impl
                     ? _impl->getPriority()
                     : MaintenancePriority());
         }
 
         MaintenanceOperation::Type getType() const {
-            return (_impl.get()
+            return (_impl
                     ? _impl->getType()
                     : MaintenanceOperation::OPERATION_COUNT);
             
@@ -132,17 +132,13 @@ public:
                 IdealStateOperation::UP operation,
                 MaintenancePriority::Priority priority);
     private:
-        Result(std::unique_ptr<ResultImpl> impl)
+        explicit Result(std::unique_ptr<ResultImpl> impl)
             : _impl(std::move(impl))
         {}
     };
 
-    /**
-     * Constructor.
-     */
-    StateChecker();
-
-    virtual ~StateChecker();
+    StateChecker() noexcept = default;
+    virtual ~StateChecker() = default;
 
     /**
      * Calculates if operations need to be scheduled to rectify any issues
@@ -150,16 +146,7 @@ public:
      *
      * @return Returns an operation to perform for the given bucket.
      */
-    virtual Result check(Context& c) = 0;
-
-    /**
-     * Used by status pages to generate human-readable information
-     * about the ideal state.
-
-     * @return Returns a string containing information about the
-     * problems this state checker is intended to solve.
-     */
-    virtual std::string getStatusText() const = 0;
+    virtual Result check(Context& c) const = 0;
 
     /**
      * Returns the name of this state checker.
diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp
index aaf65f1a6fe..9730a5f621b 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.cpp
+++ b/storage/src/vespa/storage/distributor/statecheckers.cpp
@@ -22,7 +22,7 @@ using document::BucketSpace;
 namespace storage::distributor {
 
 bool
-SplitBucketStateChecker::validForSplit(StateChecker::Context& c)
+SplitBucketStateChecker::validForSplit(Context& c)
 {
     // Can't split if we have no nodes.
     if (c.entry->getNodeCount() == 0) {
@@ -41,7 +41,7 @@ SplitBucketStateChecker::validForSplit(StateChecker::Context& c)
 }
 
 double
-SplitBucketStateChecker::getBucketSizeRelativeToMax(StateChecker::Context& c)
+SplitBucketStateChecker::getBucketSizeRelativeToMax(Context& c)
 {
     const BucketInfo& info(c.entry.getBucketInfo());
     const uint32_t highestDocumentCount(info.getHighestDocumentCount());
@@ -83,7 +83,7 @@ SplitBucketStateChecker::getBucketSizeRelativeToMax(StateChecker::Context& c)
 
 StateChecker::Result
 SplitBucketStateChecker::generateMinimumBucketSplitOperation(
-        StateChecker::Context& c)
+        Context& c)
 {
     auto so = std::make_unique<SplitOperation>(
                 c.node_ctx,
@@ -101,7 +101,7 @@ SplitBucketStateChecker::generateMinimumBucketSplitOperation(
 
 StateChecker::Result
 SplitBucketStateChecker::generateMaxSizeExceededSplitOperation(
-        StateChecker::Context& c)
+        Context& c)
 {
     auto so = std::make_unique<SplitOperation>(
                 c.node_ctx,
@@ -133,7 +133,7 @@ SplitBucketStateChecker::generateMaxSizeExceededSplitOperation(
 }
 
 StateChecker::Result
-SplitBucketStateChecker::check(StateChecker::Context& c) {
+SplitBucketStateChecker::check(Context& c) const {
     if (!validForSplit(c)) {
         return StateChecker::Result::noMaintenanceNeeded();
     }
@@ -151,7 +151,7 @@ SplitBucketStateChecker::check(StateChecker::Context& c) {
 }
 
 bool
-JoinBucketsStateChecker::isFirstSibling(const document::BucketId& bucketId) const
+JoinBucketsStateChecker::isFirstSibling(const document::BucketId& bucketId)
 {
     return (bucketId.getId() & (1ULL << (bucketId.getUsedBits() - 1))) == 0;
 }
@@ -176,8 +176,7 @@ equalNodeSet(const std::vector<uint16_t>& idealState,
 }
 
 bool
-bucketAndSiblingReplicaLocationsEqualIdealState(
-        const StateChecker::Context& context)
+bucketAndSiblingReplicaLocationsEqualIdealState(const StateChecker::Context& context)
 {
     if (!equalNodeSet(context.idealState, context.entry)) {
         return false;
@@ -207,7 +206,7 @@ inconsistentJoinIsAllowed(const StateChecker::Context& context)
 } // anon ns
 
 bool
-JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const
+JoinBucketsStateChecker::siblingsAreInSync(const Context& context)
 {
     const auto& entry(context.entry);
     const auto& siblingEntry(context.siblingEntry);
@@ -255,7 +254,7 @@ JoinBucketsStateChecker::siblingsAreInSync(const Context& context) const
 }
 
 bool
-JoinBucketsStateChecker::singleBucketJoinIsConsistent(const Context& c) const
+JoinBucketsStateChecker::singleBucketJoinIsConsistent(const Context& c)
 {
     document::BucketId joinTarget(c.getBucketId().getUsedBits() - 1,
                                   c.getBucketId().getRawId());
@@ -270,7 +269,7 @@ JoinBucketsStateChecker::singleBucketJoinIsConsistent(const Context& c) const
 }
 
 bool
-JoinBucketsStateChecker::singleBucketJoinIsEnabled(const Context& c) const
+JoinBucketsStateChecker::singleBucketJoinIsEnabled(const Context& c)
 {
     return c.distributorConfig.getEnableJoinForSiblingLessBuckets();
 }
@@ -289,8 +288,7 @@ contextBucketHasTooManyReplicas(const StateChecker::Context& c)
 }
 
 bool
-bucketAtDistributionBitLimit(const document::BucketId& bucket,
-                             const StateChecker::Context& c)
+bucketAtDistributionBitLimit(const document::BucketId& bucket, const StateChecker::Context& c)
 {
     return (bucket.getUsedBits() <= std::max(
                 uint32_t(c.systemState.getDistributionBitCount()),
@@ -300,7 +298,7 @@ bucketAtDistributionBitLimit(const document::BucketId& bucket,
 }
 
 bool
-JoinBucketsStateChecker::shouldJoin(const Context& c) const
+JoinBucketsStateChecker::shouldJoin(const Context& c)
 {
     if (c.entry->getNodeCount() == 0) {
         LOG(spam, "Not joining bucket %s because it has no nodes",
@@ -370,7 +368,7 @@ JoinBucketsStateChecker::shouldJoin(const Context& c) const
  * If sibling does not exist, treats its highest used file size as 0.
  */
 uint64_t
-JoinBucketsStateChecker::getTotalUsedFileSize(const Context& c) const
+JoinBucketsStateChecker::getTotalUsedFileSize(const Context& c)
 {
     return (c.entry.getBucketInfo().getHighestUsedFileSize()
             + c.getSiblingEntry().getBucketInfo().getHighestUsedFileSize());
@@ -381,14 +379,14 @@ JoinBucketsStateChecker::getTotalUsedFileSize(const Context& c) const
  * If sibling does not exist, treats its highest meta count as 0.
  */
 uint64_t
-JoinBucketsStateChecker::getTotalMetaCount(const Context& c) const
+JoinBucketsStateChecker::getTotalMetaCount(const Context& c)
 {
     return (c.entry.getBucketInfo().getHighestMetaCount()
             + c.getSiblingEntry().getBucketInfo().getHighestMetaCount());
 }
 
 bool
-JoinBucketsStateChecker::smallEnoughToJoin(const Context& c) const
+JoinBucketsStateChecker::smallEnoughToJoin(const Context& c)
 {
     if (c.distributorConfig.getJoinSize() != 0) {
         if (getTotalUsedFileSize(c) >= c.distributorConfig.getJoinSize()) {
@@ -406,15 +404,13 @@ JoinBucketsStateChecker::smallEnoughToJoin(const Context& c) const
 namespace {
 
 bool
-legalBucketSplitLevel(const document::BucketId& bucket,
-                      const StateChecker::Context& c)
+legalBucketSplitLevel(const document::BucketId& bucket, const StateChecker::Context& c)
 {
     return bucket.getUsedBits() >= c.distributorConfig.getMinimalBucketSplit();
 }
 
 bool
-bucketHasMultipleChildren(const document::BucketId& bucket,
-                          const StateChecker::Context& c)
+bucketHasMultipleChildren(const document::BucketId& bucket, const StateChecker::Context& c)
 {
     return c.db.childCount(bucket) > 1;
 }
@@ -422,7 +418,7 @@ bucketHasMultipleChildren(const document::BucketId& bucket,
 }
 
 document::Bucket
-JoinBucketsStateChecker::computeJoinBucket(const Context& c) const
+JoinBucketsStateChecker::computeJoinBucket(const Context& c)
 {
     // Always decrease by at least 1 bit, as we could not get here unless this
     // were a valid outcome.
@@ -444,11 +440,11 @@ JoinBucketsStateChecker::computeJoinBucket(const Context& c) const
         --level;
         target = candidate;
     }
-    return document::Bucket(c.getBucket().getBucketSpace(), target);
+    return {c.getBucket().getBucketSpace(), target};
 }
 
 StateChecker::Result
-JoinBucketsStateChecker::check(StateChecker::Context& c)
+JoinBucketsStateChecker::check(Context& c) const
 {
     // At this point in time, bucket is consistently split as the state checker
     // would otherwise be pre-empted by the inconsistent state checker.
@@ -491,14 +487,10 @@ JoinBucketsStateChecker::check(StateChecker::Context& c)
 }
 
 bool
-SplitInconsistentStateChecker::isLeastSplitBucket(
-        const document::BucketId& bucket,
-        const std::vector<BucketDatabase::Entry>& entries) const
+SplitInconsistentStateChecker::isLeastSplitBucket(const document::BucketId& bucket, const std::vector<BucketDatabase::Entry>& entries)
 {
     // Figure out if any other buckets are less split than the current one.
-    for (uint32_t i = 0; i < entries.size(); ++i) {
-        const BucketDatabase::Entry& e = entries[i];
-
+    for (const auto & e : entries) {
         assert(e.valid());
 
         if (e.getBucketId().getUsedBits() < bucket.getUsedBits()) {
@@ -510,21 +502,17 @@ SplitInconsistentStateChecker::isLeastSplitBucket(
 }
 
 uint32_t
-SplitInconsistentStateChecker::getHighestUsedBits(
-        const std::vector<BucketDatabase::Entry>& entries) const
+SplitInconsistentStateChecker::getHighestUsedBits(const std::vector<BucketDatabase::Entry>& entries)
 {
     uint32_t highestUsedBits = 0;
-    for (uint32_t i = 0; i < entries.size(); ++i) {
-        highestUsedBits = std::max(entries[i].getBucketId().getUsedBits(),
-                                   highestUsedBits);
+    for (const auto & e : entries) {
+        highestUsedBits = std::max(e.getBucketId().getUsedBits(), highestUsedBits);
     }
     return highestUsedBits;
 }
 
 vespalib::string
-SplitInconsistentStateChecker::getReason(
-        const document::BucketId& bucketId,
-        const std::vector<BucketDatabase::Entry>& entries) const
+SplitInconsistentStateChecker::getReason(const document::BucketId& bucketId, const std::vector<BucketDatabase::Entry>& entries)
 {
     vespalib::asciistream reason;
     reason << "[Bucket is inconsistently split (list includes "
@@ -560,7 +548,7 @@ isInconsistentlySplit(const StateChecker::Context& c)
 }
 
 StateChecker::Result
-SplitInconsistentStateChecker::check(StateChecker::Context& c)
+SplitInconsistentStateChecker::check(Context& c) const
 {
     if (!isInconsistentlySplit(c)) {
         return Result::noMaintenanceNeeded();
@@ -584,8 +572,7 @@ SplitInconsistentStateChecker::check(StateChecker::Context& c)
 
 namespace {
 
-bool containsMaintenanceNode(const std::vector<uint16_t>& ideal,
-                             const StateChecker::Context& c)
+bool containsMaintenanceNode(const std::vector<uint16_t>& ideal, const StateChecker::Context& c)
 {
     for (uint16_t n : ideal) {
         lib::Node node(lib::NodeType::STORAGE, n);
@@ -618,8 +605,8 @@ consistentApartFromEmptyBucketsInNonIdealLocationAndInvalidEntries(
     for (uint32_t i=0, n=entry.getNodeCount(); i<n; ++i) {
         const BucketCopy& copy(entry.getNodeRef(i));
         bool onIdealNode = false;
-        for (uint32_t j = 0; j < idealNodes.size(); ++j) {
-            if (copy.getNode() == idealNodes[j]) {
+        for (const auto & idealNode : idealNodes) {
+            if (copy.getNode() == idealNode) {
                 onIdealNode = true;
                 break;
             }
@@ -650,7 +637,7 @@ public:
         : _reason(), _nodes(), _problemFlags(0), _priority(255)
     {}
 
-    MergeNodes(const BucketDatabase::Entry& entry)
+    explicit MergeNodes(const BucketDatabase::Entry& entry)
         : _reason(), _nodes(), _problemFlags(0), _priority(255)
     {
         for (uint16_t i = 0; i < entry->getNodeCount(); i++) {
@@ -738,8 +725,7 @@ presentInIdealState(const StateChecker::Context& c, uint16_t node)
 }
 
 void
-addStatisticsForNonIdealNodes(const StateChecker::Context& c,
-                              bool missingReplica)
+addStatisticsForNonIdealNodes(const StateChecker::Context& c, bool missingReplica)
 {
     // Common case is that ideal state == actual state with no missing replicas.
     // If so, do nothing.
@@ -844,7 +830,7 @@ merging_effectively_disabled_for_state_checker(const StateChecker::Context& c) n
 }
 
 StateChecker::Result
-SynchronizeAndMoveStateChecker::check(StateChecker::Context& c)
+SynchronizeAndMoveStateChecker::check(Context& c) const
 {
     if (merging_effectively_disabled_for_state_checker(c)) {
         return Result::noMaintenanceNeeded();
@@ -899,23 +885,20 @@ SynchronizeAndMoveStateChecker::check(StateChecker::Context& c)
 }
 
 bool
-DeleteExtraCopiesStateChecker::bucketHasNoData(const StateChecker::Context& c)
+DeleteExtraCopiesStateChecker::bucketHasNoData(const Context& c)
 {
     return (c.entry->getHighestMetaCount() == 0
             && !c.entry->hasRecentlyCreatedEmptyCopy());
 }
 
 bool
-DeleteExtraCopiesStateChecker::copyIsInIdealState(const BucketCopy& cp,
-                                                  const StateChecker::Context& c) const
+DeleteExtraCopiesStateChecker::copyIsInIdealState(const BucketCopy& cp, const Context& c)
 {
     return hasItem(c.idealState, cp.getNode());
 }
 
 bool
-DeleteExtraCopiesStateChecker::enoughCopiesKept(uint32_t keptIdealCopies,
-                                                uint32_t keptNonIdealCopies,
-                                                const StateChecker::Context& c) const
+DeleteExtraCopiesStateChecker::enoughCopiesKept(uint32_t keptIdealCopies, uint32_t keptNonIdealCopies, const Context& c)
 {
     return ((keptIdealCopies + keptNonIdealCopies) >= c.distribution.getRedundancy());
 }
@@ -934,8 +917,7 @@ DeleteExtraCopiesStateChecker::addToRemoveSet(
 }
 
 uint32_t
-DeleteExtraCopiesStateChecker::numberOfIdealCopiesPresent(
-        const StateChecker::Context& c) const
+DeleteExtraCopiesStateChecker::numberOfIdealCopiesPresent(const Context& c)
 {
     const uint32_t cnt = c.entry->getNodeCount();
     uint32_t idealCopies = 0;
@@ -957,7 +939,7 @@ DeleteExtraCopiesStateChecker::numberOfIdealCopiesPresent(
  */
 void
 DeleteExtraCopiesStateChecker::removeRedundantEmptyOrConsistentCopies(
-        StateChecker::Context& c,
+        Context& c,
         std::vector<uint16_t>& removedCopies,
         vespalib::asciistream& reasons)
 {
@@ -990,7 +972,7 @@ DeleteExtraCopiesStateChecker::removeRedundantEmptyOrConsistentCopies(
 }
 
 StateChecker::Result
-DeleteExtraCopiesStateChecker::check(StateChecker::Context& c)
+DeleteExtraCopiesStateChecker::check(Context& c) const
 {
     if (c.entry->hasInvalidCopy()) {
         // Don't delete anything here.
@@ -1034,9 +1016,7 @@ DeleteExtraCopiesStateChecker::check(StateChecker::Context& c)
 }
 
 bool
-BucketStateStateChecker::shouldSkipActivationDueToMaintenance(
-        const ActiveList& activeNodes,
-        const StateChecker::Context& c) const
+BucketStateStateChecker::shouldSkipActivationDueToMaintenance(const ActiveList& activeNodes, const Context& c)
 {
     for (uint32_t i = 0; i < activeNodes.size(); ++i) {
         const auto node_index = activeNodes[i]._nodeIndex;
@@ -1067,7 +1047,7 @@ BucketStateStateChecker::shouldSkipActivationDueToMaintenance(
  *  7. Any valid copy if no copies are active
  */
 StateChecker::Result
-BucketStateStateChecker::check(StateChecker::Context& c)
+BucketStateStateChecker::check(Context& c) const
 {
     if (c.distributorConfig.isBucketActivationDisabled()) {
         return Result::noMaintenanceNeeded();
@@ -1091,7 +1071,7 @@ BucketStateStateChecker::check(StateChecker::Context& c)
     std::vector<uint16_t> operationNodes;
     for (uint32_t i=0; i<activeNodes.size(); ++i) {
         const BucketCopy* cp = c.entry->getNode(activeNodes[i]._nodeIndex);
-        if (cp == 0 || cp->active()) {
+        if (cp == nullptr || cp->active()) {
             continue;
         }
         operationNodes.push_back(activeNodes[i]._nodeIndex);
@@ -1117,7 +1097,7 @@ BucketStateStateChecker::check(StateChecker::Context& c)
         }
     }
 
-    if (operationNodes.size() == 0) {
+    if (operationNodes.empty()) {
         return Result::noMaintenanceNeeded();
     }
 
@@ -1143,13 +1123,13 @@ BucketStateStateChecker::check(StateChecker::Context& c)
 }
 
 bool
-GarbageCollectionStateChecker::garbage_collection_disabled(const Context& c) const noexcept
+GarbageCollectionStateChecker::garbage_collection_disabled(const Context& c) noexcept
 {
     return (c.distributorConfig.getGarbageCollectionInterval() == vespalib::duration::zero());
 }
 
 bool
-GarbageCollectionStateChecker::needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch) const
+GarbageCollectionStateChecker::needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch)
 {
     if (c.entry->getNodeCount() == 0) {
         return false;
@@ -1162,7 +1142,7 @@ GarbageCollectionStateChecker::needs_garbage_collection(const Context& c, std::c
 }
 
 StateChecker::Result
-GarbageCollectionStateChecker::check(Context& c)
+GarbageCollectionStateChecker::check(Context& c) const
 {
     if (garbage_collection_disabled(c)) {
         return Result::noMaintenanceNeeded();
diff --git a/storage/src/vespa/storage/distributor/statecheckers.h b/storage/src/vespa/storage/distributor/statecheckers.h
index cce66dadec6..f998dbeacfd 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.h
+++ b/storage/src/vespa/storage/distributor/statecheckers.h
@@ -8,111 +8,89 @@ namespace storage::distributor {
 class SynchronizeAndMoveStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Synchronization and moving"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "SynchronizeAndMove"; }
 };
 
 class DeleteExtraCopiesStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Delete extra copies"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "DeleteExtraCopies"; }
 
 private:
-    bool bucketHasNoData(const StateChecker::Context& c);
-    void removeRedundantEmptyOrConsistentCopies(
-            StateChecker::Context& c,
-            std::vector<uint16_t>& removedCopies,
-            vespalib::asciistream& reasons);
-    bool copyIsInIdealState(const BucketCopy& cp,
-                            const StateChecker::Context& c) const;
-    bool enoughCopiesKept(uint32_t keptIdealCopies,
-                          uint32_t keptNonIdealCopies,
-                          const StateChecker::Context& c) const;
-    uint32_t numberOfIdealCopiesPresent(const StateChecker::Context& c) const;
-    void addToRemoveSet(const BucketCopy& copyToRemove,
-                        const char* reasonForRemoval,
-                        std::vector<uint16_t>& removedCopies,
-                        vespalib::asciistream& reasons);
+    static bool bucketHasNoData(const Context& c);
+    static void removeRedundantEmptyOrConsistentCopies(Context& c, std::vector<uint16_t>& removedCopies, vespalib::asciistream& reasons);
+    static bool copyIsInIdealState(const BucketCopy& cp, const Context& c);
+    static bool enoughCopiesKept(uint32_t keptIdealCopies, uint32_t keptNonIdealCopies, const Context& c);
+    static uint32_t numberOfIdealCopiesPresent(const Context& c);
+    static void addToRemoveSet(const BucketCopy& copyToRemove, const char* reasonForRemoval,
+                               std::vector<uint16_t>& removedCopies, vespalib::asciistream& reasons);
                             
 };
 
 class JoinBucketsStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Join buckets"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "JoinBuckets"; }
 private:
-    uint64_t getTotalUsedFileSize(const Context& c) const;
-    uint64_t getTotalMetaCount(const Context& c) const;
-    bool isFirstSibling(const document::BucketId& bucketId) const;
-    bool siblingsAreInSync(const Context& c) const;
-    bool shouldJoin(const Context& c) const;
-    bool smallEnoughToJoin(const Context& c) const;
-    bool singleBucketJoinIsEnabled(const Context&) const;
-    bool singleBucketJoinIsConsistent(const Context& c) const;
-    document::Bucket computeJoinBucket(const Context& c) const;
+    static uint64_t getTotalUsedFileSize(const Context& c);
+    static uint64_t getTotalMetaCount(const Context& c);
+    static bool isFirstSibling(const document::BucketId& bucketId);
+    static bool siblingsAreInSync(const Context& c);
+    static bool shouldJoin(const Context& c);
+    static bool smallEnoughToJoin(const Context& c);
+    static bool singleBucketJoinIsEnabled(const Context&);
+    static bool singleBucketJoinIsConsistent(const Context& c);
+    static document::Bucket computeJoinBucket(const Context& c);
 };
 
 class SplitBucketStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Split buckets"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "SplitBucket"; }
 private:
-    Result generateMinimumBucketSplitOperation(Context& c);
-    Result generateMaxSizeExceededSplitOperation(Context& c);
+    static Result generateMinimumBucketSplitOperation(Context& c);
+    static Result generateMaxSizeExceededSplitOperation(Context& c);
 
-    bool validForSplit(StateChecker::Context& c);
-    double getBucketSizeRelativeToMax(Context& c);
+    static bool validForSplit(Context& c);
+    static double getBucketSizeRelativeToMax(Context& c);
 };
 
 class SplitInconsistentStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Fix inconsistently split buckets"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "SplitInconsistentBuckets"; }
 
 private:
     typedef std::pair<document::BucketId, uint16_t> BucketAndNode;
-    bool isLeastSplitBucket(
-            const document::BucketId& bucket,
-            const std::vector<BucketDatabase::Entry>& entries) const;
-    uint32_t getHighestUsedBits(
-            const std::vector<BucketDatabase::Entry>& entries) const;
-    vespalib::string getReason(
-            const document::BucketId& bucketId,
-            const std::vector<BucketDatabase::Entry>& entries) const;
-    bool isLeastSplit(Context& c, std::vector<BucketAndNode>& others);
+    static bool isLeastSplitBucket(const document::BucketId& bucket,const std::vector<BucketDatabase::Entry>& entries);
+    static uint32_t getHighestUsedBits(const std::vector<BucketDatabase::Entry>& entries);
+    static vespalib::string getReason(const document::BucketId& bucketId, const std::vector<BucketDatabase::Entry>& entries);
+    static bool isLeastSplit(Context& c, std::vector<BucketAndNode>& others);
 };
 
 class ActiveList;
 
 class BucketStateStateChecker : public StateChecker
 {
-    bool shouldSkipActivationDueToMaintenance(
-            const ActiveList& activeList,
-            const StateChecker::Context& c) const;
+    static bool shouldSkipActivationDueToMaintenance(const ActiveList& activeList, const Context& c);
 public:
-    std::string getStatusText() const override { return "Set bucket copy state"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "SetBucketState"; }
 };
 
 class GarbageCollectionStateChecker : public StateChecker
 {
 public:
-    std::string getStatusText() const override { return "Garbage collection"; }
-    Result check(Context& c) override;
+    Result check(Context& c) const override;
     const char* getName() const override { return "GarbageCollection"; }
 private:
-    [[nodiscard]] bool garbage_collection_disabled(const Context& c) const noexcept;
-    [[nodiscard]] bool needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch) const;
+    static bool garbage_collection_disabled(const Context& c) noexcept;
+    static bool needs_garbage_collection(const Context& c, std::chrono::seconds time_since_epoch);
 };
 
 }