Emit single-shot warning for phantom bucket replicas

Adding this to see if it triggers on any of our internal tests

2 files changed, 27 insertions, 15 deletions

diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp
index 5ae5d8dc3f8..eea6db8c782 100644
--- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp
+++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp
@@ -32,7 +32,8 @@ IdealStateManager::IdealStateManager(
     : HtmlStatusReporter("idealstateman", "Ideal state manager"),
       _metrics(new IdealStateMetricSet),
       _distributorComponent(owner, bucketSpaceRepo, readOnlyBucketSpaceRepo, compReg, "Ideal state manager"),
-      _bucketSpaceRepo(bucketSpaceRepo)
+      _bucketSpaceRepo(bucketSpaceRepo),
+      _has_logged_phantom_replica_warning(false)
 {
     _distributorComponent.registerStatusPage(*this);
     _distributorComponent.registerMetric(*_metrics);
@@ -52,9 +53,7 @@ IdealStateManager::IdealStateManager(
     _stateCheckers.push_back(StateChecker::SP(new GarbageCollectionStateChecker()));
 }
 
-IdealStateManager::~IdealStateManager()
-{
-}
+IdealStateManager::~IdealStateManager() = default;
 
 void
 IdealStateManager::print(std::ostream& out, bool verbose,
@@ -143,6 +142,26 @@ IdealStateManager::runStateCheckers(StateChecker::Context& c) const
     return highestPri;
 }
 
+void IdealStateManager::verify_only_live_nodes_in_context(const StateChecker::Context& c) const {
+    if (_has_logged_phantom_replica_warning) {
+        return;
+    }
+    for (const auto& n : c.entry->getRawNodes()) {
+        const uint16_t index = n.getNode();
+        const auto& state = c.systemState.getNodeState(lib::Node(lib::NodeType::STORAGE, index));
+        // Only nodes in Up, Initializing or Retired should ever be present in the DB.
+        if (!state.getState().oneOf("uir")) {
+            LOG(warning, "%s in bucket DB is on node %u, which is in unavailable state %s. "
+                         "Current cluster state is '%s'",
+                         c.entry.getBucketId().toString().c_str(),
+                         index,
+                         state.getState().toString().c_str(),
+                         c.systemState.toString().c_str());
+            _has_logged_phantom_replica_warning = true;
+        }
+    }
+}
+
 StateChecker::Result
 IdealStateManager::generateHighestPriority(
         const document::Bucket &bucket,
@@ -160,6 +179,7 @@ IdealStateManager::generateHighestPriority(
     LOG(spam, "Checking bucket %s", e->toString().c_str());
 
     c.entry = *e;
+    verify_only_live_nodes_in_context(c);
     return runStateCheckers(c);
 }
 
diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h
index 8566c67a51b..10f18a35952 100644
--- a/storage/src/vespa/storage/distributor/idealstatemanager.h
+++ b/storage/src/vespa/storage/distributor/idealstatemanager.h
@@ -41,7 +41,7 @@ public:
                       DistributorComponentRegister& compReg,
                       bool manageActiveBucketCopies);
 
-    ~IdealStateManager();
+    ~IdealStateManager() override;
 
     void print(std::ostream& out, bool verbose,
                        const std::string& indent) const;
@@ -86,6 +86,7 @@ public:
     const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _bucketSpaceRepo; }
 
 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;
     StateChecker::Result generateHighestPriority(
@@ -95,13 +96,6 @@ private:
 
     BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c) const;
 
-    friend class Operation_TestCase;
-    friend class RemoveBucketOperation_Test;
-    friend class MergeOperation_Test;
-    friend class CreateBucketOperation_Test;
-    friend class SplitOperation_Test;
-    friend class JoinOperation_Test;
-
     std::shared_ptr<IdealStateMetricSet> _metrics;
     document::BucketId _lastPrioritizedBucket;
 
@@ -112,8 +106,7 @@ private:
 
     DistributorComponent            _distributorComponent;
     DistributorBucketSpaceRepo     &_bucketSpaceRepo;
-
-    std::vector<IdealStateOperation::SP> generateOperationsForBucket(StateChecker::Context& c) const;
+    mutable bool _has_logged_phantom_replica_warning;
 
     bool iAmUp() const;
 
@@ -133,7 +126,6 @@ private:
             return true;
         }
     };
-    friend class StatusBucketVisitor;
 
     void getBucketStatus(document::BucketSpace bucketSpace, const BucketDatabase::ConstEntryRef& entry,
                          NodeMaintenanceStatsTracker& statsTracker, std::ostream& out) const;