From 443f2e69eba5694e3f05dcde7b4de050b7390f2f Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 1 Dec 2020 18:29:09 +0100 Subject: Trim down DistributorComponent member functions. --- .../src/tests/distributor/bucketdbupdatertest.cpp | 12 ++++------ .../src/tests/distributor/distributortestutil.cpp | 2 +- .../distributor/externaloperationhandlertest.cpp | 7 +++--- .../storage/distributor/distributor_bucket_space.h | 10 +++++++++ .../storage/distributor/distributorcomponent.cpp | 26 ---------------------- .../storage/distributor/distributorcomponent.h | 17 -------------- .../distributor/externaloperationhandler.cpp | 10 +++++---- 7 files changed, 25 insertions(+), 59 deletions(-) (limited to 'storage') 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 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 #include #include +#include #include #include #include @@ -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 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); -- cgit v1.2.3