From 487601a25a069a650da6ab4a6b6aaa79d7ed912c Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 5 Sep 2023 12:26:28 +0000 Subject: Constify the context and control acces to the Context._entry field --- .../src/tests/distributor/statecheckerstest.cpp | 2 +- .../storage/distributor/idealstatemanager.cpp | 65 +++------- .../vespa/storage/distributor/idealstatemanager.h | 14 +-- .../src/vespa/storage/distributor/statechecker.cpp | 30 ++++- .../src/vespa/storage/distributor/statechecker.h | 14 ++- .../vespa/storage/distributor/statecheckers.cpp | 138 ++++++++++----------- .../src/vespa/storage/distributor/statecheckers.h | 24 ++-- 7 files changed, 146 insertions(+), 141 deletions(-) diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp index 13c982f5a77..41fd8e2a48a 100644 --- a/storage/src/tests/distributor/statecheckerstest.cpp +++ b/storage/src/tests/distributor/statecheckerstest.cpp @@ -93,7 +93,7 @@ struct StateCheckersTest : Test, DistributorStripeTestUtil { // Run checking only on this bucketid, but include all buckets // owned by it or owners of it, so we can detect inconsistent split. if (entry.getBucketId() == c.getBucketId()) { - c.entry = entry; + c.entry(entry); StateChecker::Result result(checker.check(c)); IdealStateOperation::UP op(result.createOperation()); diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index bc928ca3d41..8141afd3dbf 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -49,31 +49,6 @@ IdealStateManager::print(std::ostream& out, bool verbose, const std::string& ind out << "IdealStateManager"; } -void -IdealStateManager::fillParentAndChildBuckets(StateChecker::Context& c) -{ - c.db.getAll(c.getBucketId(), c.entries); - if (c.entries.empty()) { - LOG(spam, "Did not find bucket %s in bucket database", c.bucket.toString().c_str()); - } -} -void -IdealStateManager::fillSiblingBucket(StateChecker::Context& c) -{ - c.siblingEntry = c.db.get(c.siblingBucket); -} - -BucketDatabase::Entry* -IdealStateManager::getEntryForPrimaryBucket(StateChecker::Context& c) -{ - for (auto & e : c.entries) { - if (e.getBucketId() == c.getBucketId() && ! e->getNodes().empty()) { - return &e; - } - } - return nullptr; -} - namespace { /* @@ -90,7 +65,7 @@ canOverwriteResult(const StateChecker::Result& existing, const StateChecker::Res } StateChecker::Result -IdealStateManager::runStateCheckers(StateChecker::Context& c) const +IdealStateManager::runStateCheckers(const StateChecker::Context& c) const { auto highestPri = StateChecker::Result::noMaintenanceNeeded(); // We go through _all_ active state checkers so that statistics can be @@ -114,13 +89,13 @@ IdealStateManager::verify_only_live_nodes_in_context(const StateChecker::Context if (_has_logged_phantom_replica_warning) { return; } - for (const auto& n : c.entry->getRawNodes()) { + 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(error, "%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.entry().getBucketId().toString().c_str(), index, state.getState().toString().c_str(), c.systemState.toString().c_str()); ASSERT_ONCE_OR_LOG(false, "Bucket DB contains replicas on unavailable node", 10000); _has_logged_phantom_replica_warning = true; @@ -133,16 +108,16 @@ IdealStateManager::generateHighestPriority(const document::Bucket& bucket, NodeM { auto& distributorBucketSpace = _op_ctx.bucket_space_repo().get(bucket.getBucketSpace()); StateChecker::Context c(node_context(), operation_context(), distributorBucketSpace, statsTracker, bucket); - fillParentAndChildBuckets(c); - fillSiblingBucket(c); + c.fillParentAndChildBuckets(); + c.fillSiblingBucket(); - BucketDatabase::Entry* e(getEntryForPrimaryBucket(c)); + const BucketDatabase::Entry* e(c.getEntryForPrimaryBucket()); if (!e) { return StateChecker::Result::noMaintenanceNeeded(); } LOG(spam, "Checking bucket %s", e->toString().c_str()); - c.entry = *e; + c.entry(*e); verify_only_live_nodes_in_context(c); return runStateCheckers(c); } @@ -162,23 +137,21 @@ IdealStateOperation::SP IdealStateManager::generateInterceptingSplit(BucketSpace bucketSpace, const BucketDatabase::Entry& e, api::StorageMessage::Priority pri) { + if ( ! e.valid()) return {}; + NodeMaintenanceStatsTracker statsTracker; document::Bucket bucket(bucketSpace, e.getBucketId()); auto& distributorBucketSpace = _op_ctx.bucket_space_repo().get(bucket.getBucketSpace()); StateChecker::Context c(node_context(), operation_context(), distributorBucketSpace, statsTracker, bucket); - if (e.valid()) { - c.entry = e; - - IdealStateOperation::UP operation(_splitBucketStateChecker->check(c).createOperation()); - if (operation.get()) { - operation->setPriority(pri); - operation->setIdealStateManager(this); - } + c.entry(e); - return operation; + IdealStateOperation::UP operation(_splitBucketStateChecker->check(c).createOperation()); + if (operation.get()) { + operation->setPriority(pri); + operation->setIdealStateManager(this); } - return {}; + return operation; } MaintenanceOperation::SP @@ -197,12 +170,12 @@ IdealStateManager::generateAll(const document::Bucket &bucket, NodeMaintenanceSt { auto& distributorBucketSpace = _op_ctx.bucket_space_repo().get(bucket.getBucketSpace()); StateChecker::Context c(node_context(), operation_context(), distributorBucketSpace, statsTracker, bucket); - fillParentAndChildBuckets(c); - fillSiblingBucket(c); - BucketDatabase::Entry* e(getEntryForPrimaryBucket(c)); + c.fillParentAndChildBuckets(); + c.fillSiblingBucket(); + const BucketDatabase::Entry* e(c.getEntryForPrimaryBucket()); std::vector operations; if (e) { - c.entry = *e; + c.entry(*e); } else { return operations; } diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index 39a662e4a81..949e7339fd4 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -41,15 +41,15 @@ public: static void print(std::ostream& out, bool verbose, const std::string& indent); // MaintenancePriorityGenerator interface - MaintenancePriorityAndType prioritize( - const document::Bucket& bucket, - NodeMaintenanceStatsTracker& statsTracker) const override; + MaintenancePriorityAndType prioritize(const document::Bucket& bucket, + NodeMaintenanceStatsTracker& statsTracker) const override; // MaintenanceOperationGenerator MaintenanceOperation::SP generate(const document::Bucket& bucket) const override; // MaintenanceOperationGenerator - std::vector generateAll(const document::Bucket& bucket, NodeMaintenanceStatsTracker& statsTracker) const override; + std::vector generateAll(const document::Bucket& bucket, + NodeMaintenanceStatsTracker& statsTracker) const override; /** * If the given bucket is too large, generate a split operation for it, @@ -72,12 +72,8 @@ public: private: void verify_only_live_nodes_in_context(const 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; - - static BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c); + StateChecker::Result runStateCheckers(const StateChecker::Context& c) const; IdealStateMetricSet& _metrics; std::vector _stateCheckers; diff --git a/storage/src/vespa/storage/distributor/statechecker.cpp b/storage/src/vespa/storage/distributor/statechecker.cpp index 7b30be53c13..5a85d98ddd4 100644 --- a/storage/src/vespa/storage/distributor/statechecker.cpp +++ b/storage/src/vespa/storage/distributor/statechecker.cpp @@ -78,11 +78,39 @@ StateChecker::Context::Context(const DistributorNodeContext& node_ctx_in, op_ctx(op_ctx_in), db(distributorBucketSpace.getBucketDatabase()), stats(statsTracker), - merges_inhibited_in_bucket_space(distributorBucketSpace.merges_inhibited()) + merges_inhibited_in_bucket_space(distributorBucketSpace.merges_inhibited()), + _entry() { } StateChecker::Context::~Context() = default; +void +StateChecker::Context::fillParentAndChildBuckets() +{ + db.getAll(getBucketId(), entries); + if (entries.empty()) { + LOG(spam, "Did not find bucket %s in bucket database", bucket.toString().c_str()); + } +} + +void +StateChecker::Context::fillSiblingBucket() +{ + siblingEntry = db.get(siblingBucket); +} + +const BucketDatabase::Entry* +StateChecker::Context::getEntryForPrimaryBucket() const +{ + for (auto & e : entries) { + if (e.getBucketId() == getBucketId() && ! e->getNodes().empty()) { + return &e; + } + } + return nullptr; +} + + std::string StateChecker::Context::toString() const { diff --git a/storage/src/vespa/storage/distributor/statechecker.h b/storage/src/vespa/storage/distributor/statechecker.h index d120b5e62d7..64998ca6c5f 100644 --- a/storage/src/vespa/storage/distributor/statechecker.h +++ b/storage/src/vespa/storage/distributor/statechecker.h @@ -44,8 +44,9 @@ public: * Context object used when generating operations and metrics for a * bucket. */ - struct Context + class Context { + public: Context(const DistributorNodeContext& node_ctx_in, const DistributorStripeOperationContext& op_ctx_in, const DistributorBucketSpace &distributorBucketSpace, @@ -59,7 +60,6 @@ public: // Per bucket document::Bucket bucket; document::BucketId siblingBucket; - BucketDatabase::Entry entry; BucketDatabase::Entry siblingEntry; std::vector entries; @@ -86,6 +86,14 @@ public: document::BucketSpace getBucketSpace() const noexcept { return bucket.getBucketSpace(); } std::string toString() const; + void fillParentAndChildBuckets(); + void fillSiblingBucket(); + const BucketDatabase::Entry* getEntryForPrimaryBucket() const; + const BucketDatabase::Entry & entry() const noexcept { return _entry; } + + void entry(const BucketDatabase::Entry & e) { _entry = e; } + private: + BucketDatabase::Entry _entry; }; class ResultImpl @@ -130,7 +138,7 @@ public: * * @return Returns an operation to perform for the given bucket. */ - virtual Result check(Context& c) const = 0; + virtual Result check(const 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 c26a5bc1287..20320116e79 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.cpp +++ b/storage/src/vespa/storage/distributor/statecheckers.cpp @@ -23,10 +23,10 @@ using document::BucketSpace; namespace storage::distributor { bool -SplitBucketStateChecker::validForSplit(Context& c) +SplitBucketStateChecker::validForSplit(const Context& c) { // Can't split if we have no nodes. - if (c.entry->getNodeCount() == 0) { + if (c.entry()->getNodeCount() == 0) { LOG(spam, "Can't split bucket %s, since it has no copies", c.bucket.toString().c_str()); return false; } @@ -40,9 +40,9 @@ SplitBucketStateChecker::validForSplit(Context& c) } double -SplitBucketStateChecker::getBucketSizeRelativeToMax(Context& c) +SplitBucketStateChecker::getBucketSizeRelativeToMax(const Context& c) { - auto highest = c.entry.getBucketInfo().getHighest(); + auto highest = c.entry().getBucketInfo().getHighest(); if (highest._documentCount < 2) { return 0; @@ -73,9 +73,9 @@ SplitBucketStateChecker::getBucketSizeRelativeToMax(Context& c) } StateChecker::Result -SplitBucketStateChecker::generateMinimumBucketSplitOperation(Context& c) +SplitBucketStateChecker::generateMinimumBucketSplitOperation(const Context& c) { - auto so = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry->getNodes()), + auto so = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry()->getNodes()), c.distributorConfig.getMinimalBucketSplit(), 0, 0); so->setPriority(c.distributorConfig.getMaintenancePriorities().splitDistributionBits); @@ -84,14 +84,14 @@ SplitBucketStateChecker::generateMinimumBucketSplitOperation(Context& c) } StateChecker::Result -SplitBucketStateChecker::generateMaxSizeExceededSplitOperation(Context& c) +SplitBucketStateChecker::generateMaxSizeExceededSplitOperation(const Context& c) { - auto so = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry->getNodes()), 58, + auto so = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry()->getNodes()), 58, c.distributorConfig.getSplitCount(), c.distributorConfig.getSplitSize()); so->setPriority(c.distributorConfig.getMaintenancePriorities().splitLargeBucket); - auto highest = c.entry.getBucketInfo().getHighest(); + auto highest = c.entry().getBucketInfo().getHighest(); vespalib::asciistream ost; ost << "[Splitting bucket because its maximum size (" << highest._totalDocumentSize << " b, " @@ -108,7 +108,7 @@ SplitBucketStateChecker::generateMaxSizeExceededSplitOperation(Context& c) } StateChecker::Result -SplitBucketStateChecker::check(Context& c) const { +SplitBucketStateChecker::check(const Context &c) const { if (!validForSplit(c)) { return StateChecker::Result::noMaintenanceNeeded(); } @@ -170,7 +170,7 @@ equalNodeSet(ConstNodesRef idealState, const BucketDatabase::Entry& dbEntry) bool bucketAndSiblingReplicaLocationsEqualIdealState(const StateChecker::Context& context) { - if (!equalNodeSet(context.idealStateBundle.nonretired_or_maintenance_to_index(), context.idealState(), context.entry)) { + if (!equalNodeSet(context.idealStateBundle.nonretired_or_maintenance_to_index(), context.idealState(), context.entry())) { return false; } std::vector siblingIdealState = context.distribution.getIdealStorageNodes(context.systemState, context.siblingBucket); @@ -207,7 +207,7 @@ isInconsistentlySplit(const StateChecker::Context& c) bool contextBucketHasTooManyReplicas(const StateChecker::Context& c) { - return (c.entry->getNodeCount() > c.distribution.getRedundancy()); + return (c.entry()->getNodeCount() > c.distribution.getRedundancy()); } bool @@ -234,7 +234,7 @@ bucketHasMultipleChildren(const document::BucketId& bucket, const StateChecker:: bool JoinBucketsStateChecker::siblingsAreInSync(const Context& context) { - const auto& entry(context.entry); + const auto& entry(context.entry()); const auto& siblingEntry(context.siblingEntry); if (entry->getNodeCount() != siblingEntry->getNodeCount()) { @@ -291,7 +291,7 @@ JoinBucketsStateChecker::singleBucketJoinIsEnabled(const Context& c) bool JoinBucketsStateChecker::shouldJoin(const Context& c) { - if (c.entry->getNodeCount() == 0) { + if (c.entry()->getNodeCount() == 0) { LOG(spam, "Not joining bucket %s because it has no nodes", c.bucket.toString().c_str()); return false; } @@ -312,7 +312,7 @@ JoinBucketsStateChecker::shouldJoin(const Context& c) return false; } - if (c.entry->hasRecentlyCreatedEmptyCopy()) { + if (c.entry()->hasRecentlyCreatedEmptyCopy()) { return false; } @@ -350,7 +350,7 @@ JoinBucketsStateChecker::shouldJoin(const Context& c) uint64_t JoinBucketsStateChecker::getTotalUsedFileSize(const Context& c) { - return (c.entry.getBucketInfo().getHighestUsedFileSize() + return (c.entry().getBucketInfo().getHighestUsedFileSize() + c.getSiblingEntry().getBucketInfo().getHighestUsedFileSize()); } @@ -361,7 +361,7 @@ JoinBucketsStateChecker::getTotalUsedFileSize(const Context& c) uint64_t JoinBucketsStateChecker::getTotalMetaCount(const Context& c) { - return (c.entry.getBucketInfo().getHighestMetaCount() + return (c.entry().getBucketInfo().getHighestMetaCount() + c.getSiblingEntry().getBucketInfo().getHighestMetaCount()); } @@ -408,7 +408,7 @@ JoinBucketsStateChecker::computeJoinBucket(const Context& c) } StateChecker::Result -JoinBucketsStateChecker::check(Context& c) const +JoinBucketsStateChecker::check(const 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. @@ -426,7 +426,7 @@ JoinBucketsStateChecker::check(Context& c) const sourceBuckets.push_back(c.getBucketId()); } sourceBuckets.push_back(c.getBucketId()); - auto op = std::make_unique(c.node_ctx, BucketAndNodes(joinedBucket, c.entry->getNodes()), sourceBuckets); + auto op = std::make_unique(c.node_ctx, BucketAndNodes(joinedBucket, c.entry()->getNodes()), sourceBuckets); op->setPriority(c.distributorConfig.getMaintenancePriorities().joinBuckets); vespalib::asciistream ost; ost << "[Joining buckets " << sourceBuckets[1].toString() << " and " << sourceBuckets[0].toString() @@ -487,7 +487,7 @@ SplitInconsistentStateChecker::getReason(const document::BucketId& bucketId, con } StateChecker::Result -SplitInconsistentStateChecker::check(Context& c) const +SplitInconsistentStateChecker::check(const Context &c) const { if (!isInconsistentlySplit(c)) { return Result::noMaintenanceNeeded(); @@ -497,7 +497,7 @@ SplitInconsistentStateChecker::check(Context& c) const return Result::noMaintenanceNeeded(); } - auto op = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry->getNodes()), + auto op = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry()->getNodes()), getHighestUsedBits(c.entries), 0, 0); op->setPriority(c.distributorConfig.getMaintenancePriorities().splitInconsistentBucket); @@ -657,7 +657,7 @@ MergeNodes::markMoveToIdealLocation(uint16_t node, uint8_t msgPriority) { void MergeNodes::markOutOfSync(const StateChecker::Context& c, uint8_t msgPriority) { - _reason << "[Synchronizing buckets with different checksums " << c.entry->toString() << "]"; + _reason << "[Synchronizing buckets with different checksums " << c.entry()->toString() << "]"; addProblem(OUT_OF_SYNC); updatePriority(msgPriority); } @@ -681,11 +681,11 @@ addStatisticsForNonIdealNodes(const StateChecker::Context& c, bool missingReplic { // Common case is that ideal state == actual state with no missing replicas. // If so, do nothing. - if (!missingReplica && (c.idealState().size() == c.entry->getNodeCount())) { + if (!missingReplica && (c.idealState().size() == c.entry()->getNodeCount())) { return; } - for (uint32_t j = 0; j < c.entry->getNodeCount(); ++j) { - const uint16_t node(c.entry->getNodeRef(j).getNode()); + for (uint32_t j = 0; j < c.entry()->getNodeCount(); ++j) { + const uint16_t node(c.entry()->getNodeRef(j).getNode()); if (!presentInIdealState(c, node)) { c.stats.incMovingOut(node, c.getBucketSpace()); } else if (missingReplica) { @@ -696,21 +696,21 @@ addStatisticsForNonIdealNodes(const StateChecker::Context& c, bool missingReplic } } -MergeNodes checkForNodesMissingFromIdealState(StateChecker::Context& c) __attribute__((noinline)); -MergeNodes checkIfBucketsAreOutOfSyncAndNeedMerging(StateChecker::Context& c) __attribute__((noinline)); +MergeNodes checkForNodesMissingFromIdealState(const StateChecker::Context& c) __attribute__((noinline)); +MergeNodes checkIfBucketsAreOutOfSyncAndNeedMerging(const StateChecker::Context& c) __attribute__((noinline)); MergeNodes -checkForNodesMissingFromIdealState(StateChecker::Context& c) +checkForNodesMissingFromIdealState(const StateChecker::Context& c) { MergeNodes ret; // Check if we need to add copies to get to ideal state. - if (!c.entry->emptyAndConsistent()) { + if (!c.entry()->emptyAndConsistent()) { bool hasMissingReplica = false; for (uint16_t node : c.idealState()) { bool found = false; - for (uint32_t j = 0; j < c.entry->getNodeCount(); j++) { - if (c.entry->getNodeRef(j).getNode() == node) { + for (uint32_t j = 0; j < c.entry()->getNodeCount(); j++) { + if (c.entry()->getNodeRef(j).getNode() == node) { found = true; break; } @@ -718,7 +718,7 @@ checkForNodesMissingFromIdealState(StateChecker::Context& c) if (!found) { const auto & mp = c.distributorConfig.getMaintenancePriorities(); - if (c.idealState().size() > c.entry->getNodeCount()) { + if (c.idealState().size() > c.entry()->getNodeCount()) { ret.markMissingReplica(node, mp.mergeTooFewCopies); } else { ret.markMoveToIdealLocation(node,mp.mergeMoveToIdealNode); @@ -733,20 +733,20 @@ checkForNodesMissingFromIdealState(StateChecker::Context& c) } void -addStatisticsForOutOfSyncCopies(StateChecker::Context& c) +addStatisticsForOutOfSyncCopies(const StateChecker::Context& c) { - const uint32_t n = c.entry->getNodeCount(); + const uint32_t n = c.entry()->getNodeCount(); for (uint32_t i = 0; i < n; ++i) { - const BucketCopy& cp(c.entry->getNodeRef(i)); + const BucketCopy& cp(c.entry()->getNodeRef(i)); c.stats.incSyncing(cp.getNode(), c.getBucketSpace()); } } MergeNodes -checkIfBucketsAreOutOfSyncAndNeedMerging(StateChecker::Context& c) +checkIfBucketsAreOutOfSyncAndNeedMerging(const StateChecker::Context& c) { MergeNodes ret; - if (!consistentApartFromEmptyBucketsInNonIdealLocationAndInvalidEntries(c.idealState(),c.entry.getBucketInfo())) { + if (!consistentApartFromEmptyBucketsInNonIdealLocationAndInvalidEntries(c.idealState(),c.entry().getBucketInfo())) { auto pri(c.distributorConfig.getMaintenancePriorities().mergeOutOfSyncCopies); ret.markOutOfSync(c, pri); addStatisticsForOutOfSyncCopies(c); @@ -757,9 +757,9 @@ checkIfBucketsAreOutOfSyncAndNeedMerging(StateChecker::Context& c) bool allCopiesAreInvalid(const StateChecker::Context& c) { - const uint32_t n = c.entry->getNodeCount(); + const uint32_t n = c.entry()->getNodeCount(); for (uint32_t i = 0; i < n; ++i) { - const BucketCopy& cp(c.entry->getNodeRef(i)); + const BucketCopy& cp(c.entry()->getNodeRef(i)); if (cp.valid()) { return false; } @@ -778,7 +778,7 @@ merging_effectively_disabled_for_state_checker(const StateChecker::Context& c) n } StateChecker::Result -SynchronizeAndMoveStateChecker::check(Context& c) const +SynchronizeAndMoveStateChecker::check(const Context &c) const { if (merging_effectively_disabled_for_state_checker(c)) { return Result::noMaintenanceNeeded(); @@ -796,9 +796,9 @@ SynchronizeAndMoveStateChecker::check(Context& c) const return Result::noMaintenanceNeeded(); } - assert(c.entry->getNodeCount() > 0); + assert(c.entry()->getNodeCount() > 0); - MergeNodes result(c.entry); + MergeNodes result(c.entry()); result += checkForNodesMissingFromIdealState(c); result += checkIfBucketsAreOutOfSyncAndNeedMerging(c); @@ -823,7 +823,7 @@ SynchronizeAndMoveStateChecker::check(Context& c) const return Result::createStoredResult(std::move(op), schedPri); } else { LOG(spam, "Bucket %s: No need for merge, as bucket is in consistent state (or inconsistent buckets are empty) %s", - c.bucket.toString().c_str(), c.entry->toString().c_str()); + c.bucket.toString().c_str(), c.entry()->toString().c_str()); return Result::noMaintenanceNeeded(); } } @@ -831,8 +831,8 @@ SynchronizeAndMoveStateChecker::check(Context& c) const bool DeleteExtraCopiesStateChecker::bucketHasNoData(const Context& c) { - return (c.entry->getHighestMetaCount() == 0 - && !c.entry->hasRecentlyCreatedEmptyCopy()); + return (c.entry()->getHighestMetaCount() == 0 + && !c.entry()->hasRecentlyCreatedEmptyCopy()); } bool @@ -861,10 +861,10 @@ DeleteExtraCopiesStateChecker::addToRemoveSet( uint32_t DeleteExtraCopiesStateChecker::numberOfIdealCopiesPresent(const Context& c) { - const uint32_t cnt = c.entry->getNodeCount(); + const uint32_t cnt = c.entry()->getNodeCount(); uint32_t idealCopies = 0; for (uint32_t i = 0; i < cnt; ++i) { - const BucketCopy& cp(c.entry->getNodeRef(i)); + const BucketCopy& cp(c.entry()->getNodeRef(i)); if (copyIsInIdealState(cp, c)) { ++idealCopies; } @@ -881,19 +881,19 @@ DeleteExtraCopiesStateChecker::numberOfIdealCopiesPresent(const Context& c) */ void DeleteExtraCopiesStateChecker::removeRedundantEmptyOrConsistentCopies( - Context& c, + const Context& c, std::vector& removedCopies, vespalib::asciistream& reasons) { assert(removedCopies.empty()); - const bool copiesAreConsistent = c.entry->validAndConsistent(); - const uint32_t cnt = c.entry->getNodeCount(); + const bool copiesAreConsistent = c.entry()->validAndConsistent(); + const uint32_t cnt = c.entry()->getNodeCount(); // Always keep all ideal copies uint32_t keptIdealCopies = numberOfIdealCopiesPresent(c); uint32_t keptNonIdealCopies = 0; for (uint32_t i = 0; i < cnt; ++i) { - const BucketCopy& cp(c.entry->getNodeRef(i)); + const BucketCopy& cp(c.entry()->getNodeRef(i)); if (copyIsInIdealState(cp, c)) { continue; } @@ -914,9 +914,9 @@ DeleteExtraCopiesStateChecker::removeRedundantEmptyOrConsistentCopies( } StateChecker::Result -DeleteExtraCopiesStateChecker::check(Context& c) const +DeleteExtraCopiesStateChecker::check(const Context &c) const { - if (c.entry->hasInvalidCopy()) { + if (c.entry()->hasInvalidCopy()) { // Don't delete anything here. return Result::noMaintenanceNeeded(); } @@ -930,14 +930,14 @@ DeleteExtraCopiesStateChecker::check(Context& c) const std::vector removedCopies; if (bucketHasNoData(c)) { - reasons << "[Removing all copies since bucket is empty:" << c.entry->toString() << "]"; + reasons << "[Removing all copies since bucket is empty:" << c.entry()->toString() << "]"; - for (uint32_t j = 0, cnt = c.entry->getNodeCount(); j < cnt; ++j) { - removedCopies.push_back(c.entry->getNodeRef(j).getNode()); + for (uint32_t j = 0, cnt = c.entry()->getNodeCount(); j < cnt; ++j) { + removedCopies.push_back(c.entry()->getNodeRef(j).getNode()); } - } else if (c.entry->getNodeCount() <= c.distribution.getRedundancy()) { + } else if (c.entry()->getNodeCount() <= c.distribution.getRedundancy()) { return Result::noMaintenanceNeeded(); - } else if (c.entry->hasRecentlyCreatedEmptyCopy()) { + } else if (c.entry()->hasRecentlyCreatedEmptyCopy()) { return Result::noMaintenanceNeeded(); } else { removeRedundantEmptyOrConsistentCopies(c, removedCopies, reasons); @@ -963,7 +963,7 @@ shouldSkipActivationDueToMaintenanceOrGatherOperationNodes(const ActiveList &act for (uint32_t i = 0; i < activeNodes.size(); ++i) { const ActiveCopy & active = activeNodes[i]; if ( ! active.entryIndex().valid()) continue; - const BucketCopy & cp(c.entry->getNodeRef(active.entryIndex())); + const BucketCopy & cp(c.entry()->getNodeRef(active.entryIndex())); if (cp.active()) continue; const auto node_index = active.nodeIndex(); @@ -993,7 +993,7 @@ shouldSkipActivationDueToMaintenanceOrGatherOperationNodes(const ActiveList &act * 7. Any valid copy if no copies are active */ StateChecker::Result -BucketStateStateChecker::check(Context& c) const +BucketStateStateChecker::check(const Context &c) const { if (c.distributorConfig.isBucketActivationDisabled()) { return Result::noMaintenanceNeeded(); @@ -1004,7 +1004,7 @@ BucketStateStateChecker::check(Context& c) const } ActiveList activeNodes = ActiveCopy::calculate(c.idealStateBundle.nonretired_or_maintenance_to_index(), - c.distribution, c.entry, + c.distribution, c.entry(), c.distributorConfig.max_activation_inhibited_out_of_sync_groups()); if (activeNodes.empty()) { return Result::noMaintenanceNeeded(); @@ -1024,8 +1024,8 @@ BucketStateStateChecker::check(Context& c) const } // Deactivate all copies that are currently marked as active. - for (uint32_t i = 0; i < c.entry->getNodeCount(); ++i) { - const BucketCopy& cp = c.entry->getNodeRef(i); + for (uint32_t i = 0; i < c.entry()->getNodeCount(); ++i) { + const BucketCopy& cp = c.entry()->getNodeRef(i); if (!cp.active()) { continue; } @@ -1072,32 +1072,32 @@ GarbageCollectionStateChecker::garbage_collection_disabled(const Context& c) noe bool GarbageCollectionStateChecker::needs_garbage_collection(const Context& c, vespalib::duration time_since_epoch) { - if (c.entry->getNodeCount() == 0) { + if (c.entry()->getNodeCount() == 0) { return false; } if (containsMaintenanceNode(c.idealState(), c)) { return false; } - std::chrono::seconds lastRunAt(c.entry->getLastGarbageCollectionTime()); + std::chrono::seconds lastRunAt(c.entry()->getLastGarbageCollectionTime()); return c.gcTimeCalculator.shouldGc(c.getBucketId(), time_since_epoch, lastRunAt); } StateChecker::Result -GarbageCollectionStateChecker::check(Context& c) const +GarbageCollectionStateChecker::check(const Context &c) const { if (garbage_collection_disabled(c)) { return Result::noMaintenanceNeeded(); } const vespalib::duration now(c.node_ctx.clock().getSystemTime().time_since_epoch()); - const std::chrono::seconds last_run_at(c.entry->getLastGarbageCollectionTime()); + const std::chrono::seconds last_run_at(c.entry()->getLastGarbageCollectionTime()); c.stats.update_observed_time_since_last_gc(now - last_run_at); if (needs_garbage_collection(c, now)) { - auto op = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry->getNodes())); + auto op = std::make_unique(c.node_ctx, BucketAndNodes(c.getBucket(), c.entry()->getNodes())); vespalib::asciistream reason; reason << "[Needs garbage collection: Last check at " - << c.entry->getLastGarbageCollectionTime() + << c.entry()->getLastGarbageCollectionTime() << ", current time " << vespalib::count_s(now) << ", configured interval " diff --git a/storage/src/vespa/storage/distributor/statecheckers.h b/storage/src/vespa/storage/distributor/statecheckers.h index 88cf8e9712a..e77e19e8c9d 100644 --- a/storage/src/vespa/storage/distributor/statecheckers.h +++ b/storage/src/vespa/storage/distributor/statecheckers.h @@ -8,19 +8,19 @@ namespace storage::distributor { class SynchronizeAndMoveStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "SynchronizeAndMove"; } }; class DeleteExtraCopiesStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "DeleteExtraCopies"; } private: static bool bucketHasNoData(const Context& c); - static void removeRedundantEmptyOrConsistentCopies(Context& c, std::vector& removedCopies, vespalib::asciistream& reasons); + static void removeRedundantEmptyOrConsistentCopies(const Context& c, std::vector& 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); @@ -32,7 +32,7 @@ private: class JoinBucketsStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "JoinBuckets"; } private: static uint64_t getTotalUsedFileSize(const Context& c); @@ -49,20 +49,20 @@ private: class SplitBucketStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "SplitBucket"; } private: - static Result generateMinimumBucketSplitOperation(Context& c); - static Result generateMaxSizeExceededSplitOperation(Context& c); + static Result generateMinimumBucketSplitOperation(const Context& c); + static Result generateMaxSizeExceededSplitOperation(const Context& c); - static bool validForSplit(Context& c); - static double getBucketSizeRelativeToMax(Context& c); + static bool validForSplit(const Context& c); + static double getBucketSizeRelativeToMax(const Context& c); }; class SplitInconsistentStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "SplitInconsistentBuckets"; } private: @@ -76,14 +76,14 @@ class ActiveList; class BucketStateStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "SetBucketState"; } }; class GarbageCollectionStateChecker : public StateChecker { public: - Result check(Context& c) const override; + Result check(const Context &c) const override; const char* getName() const noexcept override { return "GarbageCollection"; } private: static bool garbage_collection_disabled(const Context& c) noexcept; -- cgit v1.2.3