diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-08-11 10:36:34 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-11 10:36:34 +0200 |
commit | 4e19101d1019bc9c44ae077669e4526165387249 (patch) | |
tree | 3f3dd2dcb304954802b8a1de81ddd4c9303979be /storage/src | |
parent | 906fa79d1f612d3a7813522717bdb0d8e1ab8f39 (diff) | |
parent | 190a32ada5a9d536d72462ef91b8d5322cc950e5 (diff) |
Merge pull request #28023 from vespa-engine/balder/minor-layout-cleanup
Minor code health.
Diffstat (limited to 'storage/src')
3 files changed, 45 insertions, 132 deletions
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp index 1121c5071c0..16cc887096f 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp @@ -17,12 +17,11 @@ using document::BucketSpace; namespace storage::distributor { -DistributorStripeComponent::DistributorStripeComponent( - DistributorStripeInterface& distributor, - DistributorBucketSpaceRepo& bucketSpaceRepo, - DistributorBucketSpaceRepo& readOnlyBucketSpaceRepo, - DistributorComponentRegister& compReg, - const std::string& name) +DistributorStripeComponent::DistributorStripeComponent(DistributorStripeInterface& distributor, + DistributorBucketSpaceRepo& bucketSpaceRepo, + DistributorBucketSpaceRepo& readOnlyBucketSpaceRepo, + DistributorComponentRegister& compReg, + const std::string& name) : storage::DistributorComponent(compReg, name), _distributor(distributor), _bucketSpaceRepo(bucketSpaceRepo), @@ -44,30 +43,6 @@ DistributorStripeComponent::sendUp(const api::StorageMessage::SP& msg) _distributor.getMessageSender().sendUp(msg); } -void -DistributorStripeComponent::enumerateUnavailableNodes( - std::vector<uint16_t>& unavailableNodes, - const lib::ClusterState& s, - const document::Bucket& bucket, - const std::vector<BucketCopy>& candidates) const -{ - const auto* up_states = storage_node_up_states(); - for (uint32_t i = 0; i < candidates.size(); ++i) { - const BucketCopy& copy(candidates[i]); - const lib::NodeState& ns( - s.getNodeState(lib::Node(lib::NodeType::STORAGE, copy.getNode()))); - if (!ns.getState().oneOf(up_states)) { - LOG(debug, - "Trying to add a bucket copy to %s whose node is marked as " - "down in the cluster state: %s. Ignoring it since no zombies " - "are allowed!", - bucket.toString().c_str(), - copy.toString().c_str()); - unavailableNodes.emplace_back(copy.getNode()); - } - } -} - namespace { /** @@ -97,8 +72,7 @@ UpdateBucketDatabaseProcessor::UpdateBucketDatabaseProcessor(const framework::Cl UpdateBucketDatabaseProcessor::~UpdateBucketDatabaseProcessor() = default; BucketDatabase::Entry -UpdateBucketDatabaseProcessor::create_entry(const document::BucketId &bucket) const -{ +UpdateBucketDatabaseProcessor::create_entry(const document::BucketId &bucket) const { return BucketDatabase::Entry(bucket, BucketInfo()); } @@ -125,21 +99,16 @@ UpdateBucketDatabaseProcessor::process_entry(BucketDatabase::Entry &entry) const } void -DistributorStripeComponent::update_bucket_database( - const document::Bucket& bucket, - const std::vector<BucketCopy>& changed_nodes, - uint32_t update_flags) +DistributorStripeComponent::update_bucket_database(const document::Bucket& bucket, + const std::vector<BucketCopy>& changed_nodes, uint32_t update_flags) { auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); assert(!(bucket.getBucketId() == document::BucketId())); BucketOwnership ownership(bucketSpace.check_ownership_in_pending_and_current_state(bucket.getBucketId())); if (!ownership.isOwned()) { - LOG(debug, - "Trying to add %s to database that we do not own according to " - "cluster state '%s' - ignoring!", - bucket.toString().c_str(), - ownership.getNonOwnedState().toString().c_str()); + LOG(debug, "Trying to add %s to database that we do not own according to cluster state '%s' - ignoring!", + bucket.toString().c_str(), ownership.getNonOwnedState().toString().c_str()); return; } @@ -184,8 +153,7 @@ DistributorStripeComponent::node_address(uint16_t node_index) const noexcept // Implements DistributorStripeOperationContext void -DistributorStripeComponent::remove_nodes_from_bucket_database(const document::Bucket& bucket, - const std::vector<uint16_t>& nodes) +DistributorStripeComponent::remove_nodes_from_bucket_database(const document::Bucket& bucket, const std::vector<uint16_t>& nodes) { auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); BucketDatabase::Entry dbentry = bucketSpace.getBucketDatabase().get(bucket.getBucketId()); @@ -193,21 +161,15 @@ DistributorStripeComponent::remove_nodes_from_bucket_database(const document::Bu if (dbentry.valid()) { for (uint32_t i = 0; i < nodes.size(); ++i) { if (dbentry->removeNode(nodes[i])) { - LOG(debug, - "Removed node %d from bucket %s. %u copies remaining", - nodes[i], - bucket.toString().c_str(), - dbentry->getNodeCount()); + LOG(debug, "Removed node %d from bucket %s. %u copies remaining", + nodes[i], bucket.toString().c_str(), dbentry->getNodeCount()); } } if (dbentry->getNodeCount() != 0) { bucketSpace.getBucketDatabase().update(dbentry); } else { - LOG(debug, - "After update, bucket %s now has no copies. " - "Removing from database.", - bucket.toString().c_str()); + LOG(debug, "After update, bucket %s now has no copies. Removing from database.", bucket.toString().c_str()); bucketSpace.getBucketDatabase().remove(bucket.getBucketId()); } @@ -218,7 +180,6 @@ document::BucketId DistributorStripeComponent::make_split_bit_constrained_bucket_id(const document::DocumentId& doc_id) const { document::BucketId id(getBucketIdFactory().getBucketId(doc_id)); - id.setUsedBits(_distributor.getConfig().getMinimalBucketSplit()); return id.stripUnused(); } @@ -239,28 +200,18 @@ DistributorStripeComponent::get_sibling(const document::BucketId& bid) const zeroBucket = document::BucketId(1, 0); oneBucket = document::BucketId(1, 1); } else { - document::BucketId joinedBucket = document::BucketId( - bid.getUsedBits() - 1, - bid.getId()); - - zeroBucket = document::BucketId( - bid.getUsedBits(), - joinedBucket.getId()); - + document::BucketId joinedBucket = document::BucketId(bid.getUsedBits() - 1,bid.getId()); + zeroBucket = document::BucketId(bid.getUsedBits(), joinedBucket.getId()); uint64_t hiBit = 1; hiBit <<= (bid.getUsedBits() - 1); - oneBucket = document::BucketId( - bid.getUsedBits(), - joinedBucket.getId() | hiBit); + oneBucket = document::BucketId(bid.getUsedBits(), joinedBucket.getId() | hiBit); } return (zeroBucket == bid) ? oneBucket : zeroBucket; } bool -DistributorStripeComponent::has_pending_message(uint16_t node_index, - const document::Bucket& bucket, - uint32_t message_type) const +DistributorStripeComponent::has_pending_message(uint16_t node_index, const document::Bucket& bucket, uint32_t message_type) const { const auto& sender = static_cast<const DistributorStripeMessageSender&>(getDistributor()); return sender.getPendingMessageTracker().hasPendingMessage(node_index, bucket, message_type); @@ -275,8 +226,7 @@ DistributorStripeComponent::cluster_state_bundle() const bool DistributorStripeComponent::storage_node_is_up(document::BucketSpace bucket_space, uint32_t node_index) const { - const lib::NodeState& ns = cluster_state_bundle().getDerivedClusterState(bucket_space)->getNodeState( - lib::Node(lib::NodeType::STORAGE, node_index)); + const auto & ns = cluster_state_bundle().getDerivedClusterState(bucket_space)->getNodeState(lib::Node(lib::NodeType::STORAGE, node_index)); return ns.getState().oneOf(storage_node_up_states()); } diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_component.h b/storage/src/vespa/storage/distributor/distributor_stripe_component.h index 5bcf9eec76d..8bf507f3fac 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_component.h +++ b/storage/src/vespa/storage/distributor/distributor_stripe_component.h @@ -68,20 +68,14 @@ public: /** * Simple API for the common case of modifying a single node. */ - void update_bucket_database(const document::Bucket& bucket, - const BucketCopy& changed_node, - uint32_t update_flags) override { - update_bucket_database(bucket, - toVector<BucketCopy>(changed_node), - update_flags); + void update_bucket_database(const document::Bucket& bucket, const BucketCopy& changed_node, uint32_t update_flags) override { + update_bucket_database(bucket, toVector<BucketCopy>(changed_node),update_flags); } /** * Adds the given copies to the bucket database. */ - void update_bucket_database(const document::Bucket& bucket, - const std::vector<BucketCopy>& changed_nodes, - uint32_t update_flags) override; + void update_bucket_database(const document::Bucket& bucket, const std::vector<BucketCopy>& changed_nodes, uint32_t update_flags) override; /** * Removes a copy from the given bucket from the bucket database. @@ -97,8 +91,7 @@ public: * If the resulting bucket is empty afterwards, removes the entire * bucket entry from the bucket database. */ - void remove_nodes_from_bucket_database(const document::Bucket& bucket, - const std::vector<uint16_t>& nodes) override; + void remove_nodes_from_bucket_database(const document::Bucket& bucket, const std::vector<uint16_t>& nodes) override; const DistributorBucketSpaceRepo& bucket_space_repo() const noexcept override { return _bucketSpaceRepo; @@ -129,9 +122,7 @@ public: const DistributorConfiguration& distributor_config() const noexcept override { return getDistributor().getConfig(); } - void send_inline_split_if_bucket_too_large(document::BucketSpace bucket_space, - const BucketDatabase::Entry& entry, - uint8_t pri) override { + void send_inline_split_if_bucket_too_large(document::BucketSpace bucket_space, const BucketDatabase::Entry& entry, uint8_t pri) override { getDistributor().checkBucketForSplit(bucket_space, entry, pri); } OperationRoutingSnapshot read_snapshot_for_bucket(const document::Bucket& bucket) const override { @@ -143,9 +134,7 @@ public: const PendingMessageTracker& pending_message_tracker() const noexcept override { return getDistributor().getPendingMessageTracker(); } - bool has_pending_message(uint16_t node_index, - const document::Bucket& bucket, - uint32_t message_type) const override; + bool has_pending_message(uint16_t node_index, const document::Bucket& bucket, uint32_t message_type) const override; const lib::ClusterState* pending_cluster_state_or_null(const document::BucketSpace& bucket_space) const override { return getDistributor().pendingClusterStateOrNull(bucket_space); } @@ -171,15 +160,7 @@ public: std::unique_ptr<document::select::Node> parse_selection(const vespalib::string& selection) const override; private: - void enumerateUnavailableNodes( - std::vector<uint16_t>& unavailableNodes, - const lib::ClusterState& s, - const document::Bucket& bucket, - const std::vector<BucketCopy>& candidates) const; DistributorStripeInterface& _distributor; - -protected: - DistributorBucketSpaceRepo& _bucketSpaceRepo; DistributorBucketSpaceRepo& _readOnlyBucketSpaceRepo; }; diff --git a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp index fcd27fdab74..736d8c692e3 100644 --- a/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp +++ b/storage/src/vespa/storage/distributor/operationtargetresolverimpl.cpp @@ -23,9 +23,8 @@ make_node_list(const std::vector<uint16_t>& nodes) } -BucketInstance::BucketInstance( - const document::BucketId& id, const api::BucketInfo& info, - lib::Node node, uint16_t idealLocationPriority, bool trusted, bool exist) +BucketInstance::BucketInstance(const document::BucketId& id, const api::BucketInfo& info, lib::Node node, + uint16_t idealLocationPriority, bool trusted, bool exist) : _bucket(id), _info(info), _node(node), _idealLocationPriority(idealLocationPriority), _trusted(trusted), _exist(exist) { @@ -39,12 +38,8 @@ BucketInstance::print(vespalib::asciistream& out, const PrintProperties&) const std::ostringstream ost; ost << std::hex << _bucket.getId(); - out << "(" << ost.str() << ", " - << infoString << ", node " << _node.getIndex() - << ", ideal " << _idealLocationPriority - << (_trusted ? ", trusted" : "") - << (_exist ? "" : ", new copy") - << ")"; + out << "(" << ost.str() << ", " << infoString << ", node " << _node.getIndex() << ", ideal " << _idealLocationPriority + << (_trusted ? ", trusted" : "") << (_exist ? "" : ", new copy") << ")"; } bool @@ -71,8 +66,7 @@ BucketInstanceList::populate(const document::BucketId& specificId, const Distrib std::vector<BucketDatabase::Entry> entries; db.getParents(specificId, entries); for (const auto & entry : entries) { - lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle( - entry.getBucketId()).available_nonretired_or_maintenance_nodes())); + lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(entry.getBucketId()).available_nonretired_or_maintenance_nodes())); add(entry, idealNodes); } } @@ -100,41 +94,34 @@ BucketInstanceList::limitToRedundancyCopies(uint16_t redundancy) } document::BucketId -BucketInstanceList::leastSpecificLeafBucketInSubtree( - const document::BucketId& candidateId, - const document::BucketId& mostSpecificId, - const BucketDatabase& db) const +BucketInstanceList::leastSpecificLeafBucketInSubtree(const document::BucketId& candidateId, + const document::BucketId& mostSpecificId, + const BucketDatabase& db) const { assert(candidateId.contains(mostSpecificId)); document::BucketId treeNode = candidateId; // treeNode may reach at most 58 bits since buckets at 58 bits by definition // cannot have any children. while (db.childCount(treeNode) != 0) { - treeNode = document::BucketId(treeNode.getUsedBits() + 1, - mostSpecificId.getRawId()); + treeNode = document::BucketId(treeNode.getUsedBits() + 1, mostSpecificId.getRawId()); } assert(treeNode.contains(mostSpecificId)); return treeNode; } void -BucketInstanceList::extendToEnoughCopies( - const DistributorBucketSpace& distributor_bucket_space, - const BucketDatabase& db, - const document::BucketId& targetIfNonPreExisting, - const document::BucketId& mostSpecificId) +BucketInstanceList::extendToEnoughCopies(const DistributorBucketSpace& distributor_bucket_space, + const BucketDatabase& db, + const document::BucketId& targetIfNonPreExisting, + const document::BucketId& mostSpecificId) { - document::BucketId newTarget(_instances.empty() ? targetIfNonPreExisting - : _instances[0]._bucket); + document::BucketId newTarget(_instances.empty() ? targetIfNonPreExisting : _instances[0]._bucket); newTarget = leastSpecificLeafBucketInSubtree(newTarget, mostSpecificId, db); - lib::IdealNodeList idealNodes(make_node_list( - distributor_bucket_space.get_ideal_service_layer_nodes_bundle(newTarget).available_nonretired_nodes())); + lib::IdealNodeList idealNodes(make_node_list(distributor_bucket_space.get_ideal_service_layer_nodes_bundle(newTarget).available_nonretired_nodes())); for (uint32_t i=0; i<idealNodes.size(); ++i) { if (!contains(idealNodes[i])) { - _instances.push_back(BucketInstance( - newTarget, api::BucketInfo(), idealNodes[i], - i, false, false)); + _instances.emplace_back(newTarget, api::BucketInfo(), idealNodes[i], i, false, false); } } } @@ -187,22 +174,17 @@ struct InstanceOrder { } // anonymous BucketInstanceList -OperationTargetResolverImpl::getAllInstances(OperationType type, - const document::BucketId& id) +OperationTargetResolverImpl::getAllInstances(OperationType type, const document::BucketId& id) { BucketInstanceList instances; if (type == PUT) { instances.populate(id, _distributor_bucket_space, _bucketDatabase); instances.sort(InstanceOrder()); instances.removeNodeDuplicates(); - instances.extendToEnoughCopies( - _distributor_bucket_space, - _bucketDatabase, - _bucketDatabase.getAppropriateBucket(_minUsedBucketBits, id), - id); + instances.extendToEnoughCopies(_distributor_bucket_space, _bucketDatabase, + _bucketDatabase.getAppropriateBucket(_minUsedBucketBits, id), id); } else { - throw vespalib::IllegalArgumentException( - "Unsupported operation type given", VESPA_STRLOC); + throw vespalib::IllegalArgumentException("Unsupported operation type given", VESPA_STRLOC); } return instances; } |