Trim down DistributorComponent member functions.

7 files changed, 25 insertions, 59 deletions

diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp
index e42f024b730..108368fff7f 100644
--- a/storage/src/tests/distributor/bucketdbupdatertest.cpp
+++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp
@@ -144,8 +144,7 @@ public:
         api::RequestBucketInfoReply::EntryVector &vec = sreply->getBucketInfo();
 
         for (uint32_t i=0; i<bucketCount + invalidBucketCount; i++) {
-            if (!getBucketDBUpdater().getDistributorComponent()
-                .ownsBucketInState(state, makeDocumentBucket(document::BucketId(16, i)))) {
+            if (!getDistributorBucketSpace().owns_bucket_in_state(state, document::BucketId(16, i))) {
                 continue;
             }
 
@@ -1779,8 +1778,7 @@ TEST_F(BucketDBUpdaterTest, no_db_resurrection_for_bucket_not_owned_in_current_s
         uint32_t expectedMsgs = messageCount(2), dummyBucketsToReturn = 1;
         ASSERT_NO_FATAL_FAILURE(setAndEnableClusterState(stateAfter, expectedMsgs, dummyBucketsToReturn));
     }
-    EXPECT_FALSE(getBucketDBUpdater().getDistributorComponent()
-                 .ownsBucketInCurrentState(makeDocumentBucket(bucket)));
+    EXPECT_FALSE(getDistributorBucketSpace().owns_bucket_in_current_state(bucket));
 
     ASSERT_NO_FATAL_FAILURE(sendFakeReplyForSingleBucketRequest(*rbi));
 
@@ -1806,8 +1804,7 @@ TEST_F(BucketDBUpdaterTest, no_db_resurrection_for_bucket_not_owned_in_pending_s
     lib::ClusterState stateAfter("distributor:3 storage:3");
     // Set, but _don't_ enable cluster state. We want it to be pending.
     setSystemState(stateAfter);
-    EXPECT_TRUE(getBucketDBUpdater().getDistributorComponent()
-                .ownsBucketInCurrentState(makeDocumentBucket(bucket)));
+    EXPECT_TRUE(getDistributorBucketSpace().owns_bucket_in_current_state(bucket));
     EXPECT_FALSE(getBucketDBUpdater()
                  .checkOwnershipInPendingState(makeDocumentBucket(bucket)).isOwned());
 
@@ -2390,8 +2387,7 @@ TEST_F(BucketDBUpdaterTest, non_owned_buckets_moved_to_read_only_db_on_ownership
 
     std::unordered_set<Bucket, Bucket::hash> buckets_not_owned_in_pending_state;
     for_each_bucket(mutable_repo(), [&](const auto& space, const auto& entry) {
-        if (!getBucketDBUpdater().getDistributorComponent()
-                .ownsBucketInState(pending_state, makeDocumentBucket(entry.getBucketId()))) {
+        if (!getDistributorBucketSpace().owns_bucket_in_state(pending_state, entry.getBucketId())) {
             buckets_not_owned_in_pending_state.insert(Bucket(space, entry.getBucketId()));
         }
     });
diff --git a/storage/src/tests/distributor/distributortestutil.cpp b/storage/src/tests/distributor/distributortestutil.cpp
index 3c26ea01c19..17932ccffd7 100644
--- a/storage/src/tests/distributor/distributortestutil.cpp
+++ b/storage/src/tests/distributor/distributortestutil.cpp
@@ -133,7 +133,7 @@ DistributorTestUtil::getNodes(document::BucketId id)
 std::string
 DistributorTestUtil::getIdealStr(document::BucketId id, const lib::ClusterState& state)
 {
-    if (!getExternalOperationHandler().ownsBucketInState(state, makeDocumentBucket(id))) {
+    if (!getDistributorBucketSpace().owns_bucket_in_state(state, id)) {
         return id.toString();
     }
 
diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp
index 9bd64876e98..e628b632215 100644
--- a/storage/src/tests/distributor/externaloperationhandlertest.cpp
+++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp
@@ -3,6 +3,7 @@
 #include <tests/distributor/distributortestutil.h>
 #include <vespa/storage/distributor/externaloperationhandler.h>
 #include <vespa/storage/distributor/distributor.h>
+#include <vespa/storage/distributor/distributor_bucket_space.h>
 #include <vespa/storage/distributor/distributormetricsset.h>
 #include <vespa/storage/distributor/operations/external/getoperation.h>
 #include <vespa/storageapi/message/persistence.h>
@@ -129,7 +130,7 @@ ExternalOperationHandlerTest::findNonOwnedUserBucketInState(
     lib::ClusterState state(statestr);
     for (uint64_t i = 1; i < 1000; ++i) {
         document::BucketId bucket(32, i);
-        if (!getExternalOperationHandler().ownsBucketInState(state, makeDocumentBucket(bucket))) {
+        if (!getDistributorBucketSpace().owns_bucket_in_state(state, bucket)) {
             return bucket;
         }
     }
@@ -145,8 +146,8 @@ ExternalOperationHandlerTest::findOwned1stNotOwned2ndInStates(
     lib::ClusterState state2(statestr2);
     for (uint64_t i = 1; i < 1000; ++i) {
         document::BucketId bucket(32, i);
-        if (getExternalOperationHandler().ownsBucketInState(state1, makeDocumentBucket(bucket))
-            && !getExternalOperationHandler().ownsBucketInState(state2, makeDocumentBucket(bucket)))
+        if (getDistributorBucketSpace().owns_bucket_in_state(state1, bucket)
+            && !getDistributorBucketSpace().owns_bucket_in_state(state2, bucket))
         {
             return bucket;
         }
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
index 96440fbabad..aada7999316 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
@@ -81,7 +81,17 @@ public:
     std::vector<uint16_t> get_ideal_nodes_fallback(document::BucketId bucket) const;
 
     bool owns_bucket_in_state(const lib::Distribution& distribution, const lib::ClusterState& cluster_state, document::BucketId bucket) const;
+
+    /**
+     * Returns true if this distributor owns the given bucket in the
+     * given cluster and current distribution config.
+     */
     bool owns_bucket_in_state(const lib::ClusterState& clusterState, document::BucketId bucket) const;
+
+    /**
+     * Returns true if this distributor owns the given bucket with the current
+     * cluster state and distribution config.
+     */
     bool owns_bucket_in_current_state(document::BucketId bucket) const;
 
     BucketOwnership check_ownership_in_pending_state(document::BucketId bucket) const;
diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.cpp b/storage/src/vespa/storage/distributor/distributorcomponent.cpp
index a9247deefe8..aa0ae42bf9d 100644
--- a/storage/src/vespa/storage/distributor/distributorcomponent.cpp
+++ b/storage/src/vespa/storage/distributor/distributorcomponent.cpp
@@ -63,32 +63,6 @@ DistributorComponent::checkOwnershipInPendingAndCurrentState(
     return bucket_space.check_ownership_in_pending_and_current_state(bucket.getBucketId());
 }
 
-bool
-DistributorComponent::ownsBucketInState(
-        const lib::Distribution& distribution,
-        const lib::ClusterState& clusterState,
-        const document::Bucket &bucket) const
-{
-    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
-    return bucket_space.owns_bucket_in_state(distribution, clusterState, bucket.getBucketId());
-}
-
-bool
-DistributorComponent::ownsBucketInState(
-        const lib::ClusterState& clusterState,
-        const document::Bucket &bucket) const
-{
-    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
-    return bucket_space.owns_bucket_in_state(clusterState, bucket.getBucketId());
-}
-
-bool
-DistributorComponent::ownsBucketInCurrentState(const document::Bucket &bucket) const
-{
-    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
-    return bucket_space.owns_bucket_in_current_state(bucket.getBucketId());
-}
-
 api::StorageMessageAddress
 DistributorComponent::nodeAddress(uint16_t nodeIndex) const
 {
diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.h b/storage/src/vespa/storage/distributor/distributorcomponent.h
index 0d0449836bb..805ecd5c7e3 100644
--- a/storage/src/vespa/storage/distributor/distributorcomponent.h
+++ b/storage/src/vespa/storage/distributor/distributorcomponent.h
@@ -50,23 +50,6 @@ public:
     BucketOwnership checkOwnershipInPendingAndCurrentState(
             const document::Bucket &bucket) const;
 
-    bool ownsBucketInState(const lib::Distribution& distribution,
-                           const lib::ClusterState& clusterState,
-                           const document::Bucket &bucket) const;
-
-    /**
-     * Returns true if this distributor owns the given bucket in the
-     * given cluster and current distribution config.
-     */
-    bool ownsBucketInState(const lib::ClusterState& clusterState,
-                           const document::Bucket &bucket) const;
-
-    /**
-     * Returns true if this distributor owns the given bucket with the current
-     * cluster state and distribution config.
-     */
-    bool ownsBucketInCurrentState(const document::Bucket &bucket) const;
-
     /**
      * Returns a reference to the current cluster state bundle. Valid until the
      * next time the distributor main thread processes its message queue.
diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
index e3a6056a723..980374686d9 100644
--- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
+++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp
@@ -161,8 +161,9 @@ ExternalOperationHandler::checkTimestampMutationPreconditions(api::StorageComman
                                                               const document::BucketId &bucketId,
                                                               PersistenceOperationMetricSet& persistenceMetrics)
 {
-    document::Bucket bucket(cmd.getBucket().getBucketSpace(), bucketId);
-    if (!ownsBucketInCurrentState(bucket)) {
+    auto &bucket_space(_bucketSpaceRepo.get(cmd.getBucket().getBucketSpace()));
+    if (!bucket_space.owns_bucket_in_current_state(bucketId)) {
+        document::Bucket bucket(cmd.getBucket().getBucketSpace(), bucketId);
         LOG(debug, "Distributor manager received %s, bucket %s with wrong distribution",
             cmd.toString().c_str(), bucket.toString().c_str());
         bounce_with_wrong_distribution(cmd);
@@ -170,7 +171,7 @@ ExternalOperationHandler::checkTimestampMutationPreconditions(api::StorageComman
         return false;
     }
 
-    auto pending = getDistributor().checkOwnershipInPendingState(bucket);
+    auto pending = bucket_space.check_ownership_in_pending_state(bucketId);
     if (!pending.isOwned()) {
         // We return BUSY here instead of WrongDistributionReply to avoid clients potentially
         // ping-ponging between cluster state versions during a state transition.
@@ -217,7 +218,8 @@ void ExternalOperationHandler::bounce_or_invoke_read_only_op(
         PersistenceOperationMetricSet& metrics,
         Func func)
 {
-    if (!ownsBucketInCurrentState(bucket)) {
+    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
+    if (!bucket_space.owns_bucket_in_current_state(bucket.getBucketId())) {
         LOG(debug, "Distributor manager received %s, bucket %s with wrong distribution",
             cmd.toString().c_str(), bucket.toString().c_str());
         bounce_with_wrong_distribution(cmd);