Merge pull request #15594 from vespa-engine/toregge/remove-distributor-component-trampoline-member-function

Remove DistributorComponent trampoline member function.

9 files changed, 27 insertions, 46 deletions

diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp
index 3c80bfd3881..93f503c27e4 100644
--- a/storage/src/tests/distributor/distributortest.cpp
+++ b/storage/src/tests/distributor/distributortest.cpp
@@ -558,9 +558,7 @@ TEST_F(DistributorTest, no_db_resurrection_for_bucket_not_owned_in_pending_state
 
     document::BucketId nonOwnedBucket(16, 3);
     EXPECT_FALSE(getDistributorBucketSpace().check_ownership_in_pending_state(nonOwnedBucket).isOwned());
-    EXPECT_FALSE(getBucketDBUpdater().getDistributorComponent()
-                     .checkOwnershipInPendingAndCurrentState(makeDocumentBucket(nonOwnedBucket))
-                     .isOwned());
+    EXPECT_FALSE(getDistributorBucketSpace().check_ownership_in_pending_and_current_state(nonOwnedBucket).isOwned());
 
     std::vector<BucketCopy> copies;
     copies.emplace_back(1234, 0, api::BucketInfo(0x567, 1, 2));
diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp
index 1515eba4bf0..e86eded7392 100644
--- a/storage/src/tests/distributor/putoperationtest.cpp
+++ b/storage/src/tests/distributor/putoperationtest.cpp
@@ -3,6 +3,7 @@
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/storage/distributor/operations/external/putoperation.h>
 #include <vespa/storage/distributor/distributor.h>
+#include <vespa/storage/distributor/distributor_bucket_space.h>
 #include <vespa/storageapi/message/bucket.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <vespa/storageapi/message/state.h>
@@ -483,7 +484,7 @@ PutOperationTest::getNodes(const std::string& infoString) {
 
     std::vector<uint16_t> targetNodes;
     std::vector<uint16_t> createNodes;
-    PutOperation::getTargetNodes(getExternalOperationHandler().getIdealNodes(makeDocumentBucket(bid)),
+    PutOperation::getTargetNodes(getDistributorBucketSpace().get_ideal_nodes(bid),
                                  targetNodes, createNodes, entry, 2);
 
     ost << "target( ";
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
index 111e58045a1..82bbeadb9a5 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp
@@ -168,7 +168,7 @@ DistributorBucketSpace::check_ownership_in_pending_and_current_state_fallback(do
 }
 
 std::vector<uint16_t>
-DistributorBucketSpace::get_ideal_nodes(document::BucketId bucket)
+DistributorBucketSpace::get_ideal_nodes(document::BucketId bucket) const
 {
     assert(bucket.getUsedBits() >= _distribution_bits);
     if (bucket.getUsedBits() > 33) { // cf. storage::lib::Distribution::getStorageSeed
@@ -186,7 +186,7 @@ DistributorBucketSpace::get_ideal_nodes(document::BucketId bucket)
 }
 
 BucketOwnership
-DistributorBucketSpace::check_ownership_in_pending_and_current_state(document::BucketId bucket)
+DistributorBucketSpace::check_ownership_in_pending_and_current_state(document::BucketId bucket) const
 {
     if (bucket.getUsedBits() < _distribution_bits) {
         // Cannot map to super bucket ==> cannot cache result
diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
index 4044f7c5119..96536b26da3 100644
--- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h
+++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h
@@ -37,8 +37,8 @@ class DistributorBucketSpace {
     uint16_t                                 _distribution_bits;
     std::shared_ptr<const lib::ClusterState> _pending_cluster_state;
     std::vector<bool>                        _available_nodes;
-    vespalib::hash_map<document::BucketId, BucketOwnership, document::BucketId::hash>       _ownerships;
-    vespalib::hash_map<document::BucketId, std::vector<uint16_t>, document::BucketId::hash> _ideal_nodes;
+    mutable vespalib::hash_map<document::BucketId, BucketOwnership, document::BucketId::hash>       _ownerships;
+    mutable vespalib::hash_map<document::BucketId, std::vector<uint16_t>, document::BucketId::hash> _ideal_nodes;
 
     void clear();
     void enumerate_available_nodes();
@@ -99,13 +99,26 @@ public:
      * Otherwise always returns "is owned", i.e. it must also be checked in the current state.
      */
     BucketOwnership check_ownership_in_pending_state(document::BucketId bucket) const;
+    /**
+     * Returns the ownership status of a bucket as decided with the given
+     * distribution and cluster state -and- that of the pending cluster
+     * state and distribution (if any pending exists).
+     */
     BucketOwnership check_ownership_in_pending_and_given_state(const lib::Distribution& distribution,
                                                                const lib::ClusterState& clusterState,
                                                                document::BucketId bucket) const;
     BucketOwnership check_ownership_in_pending_and_current_state_fallback(document::BucketId bucket) const;
     const std::vector<bool>& get_available_nodes() const { return _available_nodes; }
-    std::vector<uint16_t> get_ideal_nodes(document::BucketId bucket);
-    BucketOwnership check_ownership_in_pending_and_current_state(document::BucketId bucket);
+    /**
+     * Returns the ideal nodes for the given bucket.
+     */
+    std::vector<uint16_t> get_ideal_nodes(document::BucketId bucket) const;
+    /**
+     * Returns the ownership status of a bucket as decided with the current
+     * distribution and cluster state -and- that of the pending cluster
+     * state and distribution (if any pending exists).
+     */
+    BucketOwnership check_ownership_in_pending_and_current_state(document::BucketId bucket) const;
 };
 
 }
diff --git a/storage/src/vespa/storage/distributor/distributor_operation_context.h b/storage/src/vespa/storage/distributor/distributor_operation_context.h
index e4832a7db12..9b2c46f0071 100644
--- a/storage/src/vespa/storage/distributor/distributor_operation_context.h
+++ b/storage/src/vespa/storage/distributor/distributor_operation_context.h
@@ -23,7 +23,6 @@ class DistributorOperationContext {
 public:
     virtual ~DistributorOperationContext() {}
     virtual api::Timestamp generate_unique_timestamp() = 0;
-    virtual BucketOwnership check_ownership_in_pending_and_current_state(const document::Bucket &bucket) const = 0;
     virtual void update_bucket_database(const document::Bucket& bucket,
                                         const BucketCopy& changed_node,
                                         uint32_t update_flags = 0) = 0;
diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.cpp b/storage/src/vespa/storage/distributor/distributorcomponent.cpp
index aa0ae42bf9d..41a99092174 100644
--- a/storage/src/vespa/storage/distributor/distributorcomponent.cpp
+++ b/storage/src/vespa/storage/distributor/distributorcomponent.cpp
@@ -48,21 +48,6 @@ DistributorComponent::getClusterStateBundle() const
     return _distributor.getClusterStateBundle();
 };
 
-std::vector<uint16_t>
-DistributorComponent::getIdealNodes(const document::Bucket &bucket) const
-{
-    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
-    return bucket_space.get_ideal_nodes(bucket.getBucketId());
-}
-
-BucketOwnership
-DistributorComponent::checkOwnershipInPendingAndCurrentState(
-        const document::Bucket &bucket) const
-{
-    auto &bucket_space(_bucketSpaceRepo.get(bucket.getBucketSpace()));
-    return bucket_space.check_ownership_in_pending_and_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 805ecd5c7e3..d7fffaaf271 100644
--- a/storage/src/vespa/storage/distributor/distributorcomponent.h
+++ b/storage/src/vespa/storage/distributor/distributorcomponent.h
@@ -43,25 +43,12 @@ public:
     ~DistributorComponent() override;
 
     /**
-     * Returns the ownership status of a bucket as decided with the given
-     * distribution and cluster state -and- that of the pending cluster
-     * state and distribution (if any pending exists).
-     */
-    BucketOwnership checkOwnershipInPendingAndCurrentState(
-            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.
      */
     const lib::ClusterStateBundle& getClusterStateBundle() const;
 
     /**
-     * Returns the ideal nodes for the given bucket.
-     */
-    std::vector<uint16_t> getIdealNodes(const document::Bucket &bucket) const;
-
-    /**
       * Returns the slobrok address of the given storage node.
       */
     api::StorageMessageAddress nodeAddress(uint16_t nodeIndex) const;
@@ -166,9 +153,6 @@ public:
 
     // Implements DistributorOperationContext
     api::Timestamp generate_unique_timestamp() override { return getUniqueTimestamp(); }
-    BucketOwnership check_ownership_in_pending_and_current_state(const document::Bucket &bucket) const override {
-        return checkOwnershipInPendingAndCurrentState(bucket);
-    }
     void update_bucket_database(const document::Bucket& bucket,
                                 const BucketCopy& changed_node,
                                 uint32_t update_flags = 0) override {
diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
index 72193c99a64..46eb104db56 100644
--- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
@@ -5,6 +5,7 @@
 #include "putoperation.h"
 #include "updateoperation.h"
 #include <vespa/storage/distributor/distributor_bucket_space.h>
+#include <vespa/storage/distributor/distributor_bucket_space_repo.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <vespa/document/datatype/documenttype.h>
 #include <vespa/document/fieldvalue/document.h>
@@ -251,8 +252,8 @@ TwoPhaseUpdateOperation::onStart(DistributorMessageSender& sender) {
 bool
 TwoPhaseUpdateOperation::lostBucketOwnershipBetweenPhases() const
 {
-    document::Bucket updateDocBucket(_updateCmd->getBucket().getBucketSpace(), _updateDocBucketId);
-    BucketOwnership bo(_op_ctx.check_ownership_in_pending_and_current_state(updateDocBucket));
+    auto &bucket_space(_op_ctx.bucket_space_repo().get(_updateCmd->getBucket().getBucketSpace()));
+    BucketOwnership bo(bucket_space.check_ownership_in_pending_and_current_state(_updateDocBucketId));
     return !bo.isOwned();
 }
 
diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
index cf6922b8606..3d79aa176d7 100644
--- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
@@ -297,8 +297,8 @@ VisitorOperation::verifyDistributorIsNotDown(const lib::ClusterState& state)
 void
 VisitorOperation::verifyDistributorOwnsBucket(const document::BucketId& bid)
 {
-    document::Bucket bucket(_msg->getBucketSpace(), bid);
-    BucketOwnership bo(_op_ctx.check_ownership_in_pending_and_current_state(bucket));
+    auto &bucket_space(_op_ctx.bucket_space_repo().get(_msg->getBucketSpace()));
+    BucketOwnership bo(bucket_space.check_ownership_in_pending_and_current_state(bid));
     if (!bo.isOwned()) {
         verifyDistributorIsNotDown(bo.getNonOwnedState());
         std::string systemStateStr = bo.getNonOwnedState().toString();