diff options
Diffstat (limited to 'storage')
6 files changed, 60 insertions, 40 deletions
diff --git a/storage/src/tests/distributor/CMakeLists.txt b/storage/src/tests/distributor/CMakeLists.txt index c59b56eb68f..11bbf2c241a 100644 --- a/storage/src/tests/distributor/CMakeLists.txt +++ b/storage/src/tests/distributor/CMakeLists.txt @@ -19,7 +19,6 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST externaloperationhandlertest.cpp garbagecollectiontest.cpp getoperationtest.cpp - gtest_runner.cpp idealstatemanagertest.cpp joinbuckettest.cpp maintenancemocks.cpp @@ -27,6 +26,7 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST mergelimitertest.cpp mergeoperationtest.cpp multi_thread_stripe_access_guard_test.cpp + newest_replica_test.cpp node_supported_features_repo_test.cpp nodeinfotest.cpp nodemaintenancestatstrackertest.cpp @@ -57,7 +57,7 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST storage_testcommon storage_testhostreporter storage - GTest::GTest + GTest::gmock_main ) vespa_add_test( diff --git a/storage/src/tests/distributor/gtest_runner.cpp b/storage/src/tests/distributor/gtest_runner.cpp deleted file mode 100644 index 7c20f681093..00000000000 --- a/storage/src/tests/distributor/gtest_runner.cpp +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include <vespa/vespalib/gtest/gtest.h> - -#include <vespa/log/log.h> -LOG_SETUP("storage_distributor_gtest_runner"); - -GTEST_MAIN_RUN_ALL_TESTS() diff --git a/storage/src/tests/distributor/newest_replica_test.cpp b/storage/src/tests/distributor/newest_replica_test.cpp new file mode 100644 index 00000000000..9267c6e37a2 --- /dev/null +++ b/storage/src/tests/distributor/newest_replica_test.cpp @@ -0,0 +1,24 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/storage/distributor/operations/external/newest_replica.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/gtest/matchers/elements_are_distinct.h> + +using namespace ::testing; +using storage::api::Timestamp; +using document::BucketId; + +namespace storage::distributor { + +TEST(NewestReplicaTest, equality_predicate_considers_all_fields) { + std::vector elems = { + NewestReplica::of(Timestamp(1000), BucketId(16, 1), 0, false, false), + NewestReplica::of(Timestamp(1001), BucketId(16, 1), 0, false, false), + NewestReplica::of(Timestamp(1000), BucketId(16, 2), 0, false, false), + NewestReplica::of(Timestamp(1000), BucketId(16, 1), 1, false, false), + NewestReplica::of(Timestamp(1000), BucketId(16, 1), 0, true, false), + NewestReplica::of(Timestamp(1000), BucketId(16, 1), 0, false, true) + }; + EXPECT_THAT(elems, ElementsAreDistinct()); +} + +} diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp index dea215f47dc..aa5ad217ae9 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.cpp @@ -15,13 +15,12 @@ #include <algorithm> #include <vespa/log/log.h> -LOG_SETUP(".distributor.callback.doc.put"); +LOG_SETUP(".distributor.operations.external.put"); - -using namespace storage::distributor; -using namespace storage; using document::BucketSpace; +namespace storage::distributor { + PutOperation::PutOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, DistributorBucketSpace& bucketSpace, @@ -116,6 +115,20 @@ bool PutOperation::has_unavailable_targets_in_pending_state(const OperationTarge }); } +bool PutOperation::at_least_one_storage_node_is_available() const { + const lib::ClusterState& cluster_state = _bucketSpace.getClusterState(); + + const uint16_t storage_node_index_ubound = cluster_state.getNodeCount(lib::NodeType::STORAGE); + for (uint16_t i = 0; i < storage_node_index_ubound; i++) { + if (cluster_state.getNodeState(lib::Node(lib::NodeType::STORAGE, i)) + .getState().oneOf(storage_node_up_states())) + { + return true; + } + } + return false; +} + void PutOperation::onStart(DistributorStripeMessageSender& sender) { @@ -124,19 +137,7 @@ PutOperation::onStart(DistributorStripeMessageSender& sender) LOG(debug, "Received PUT %s for bucket %s", _msg->getDocumentId().toString().c_str(), bid.toString().c_str()); - lib::ClusterState systemState = _bucketSpace.getClusterState(); - - // Don't do anything if all nodes are down. - bool up = false; - for (uint16_t i = 0; i < systemState.getNodeCount(lib::NodeType::STORAGE); i++) { - if (systemState.getNodeState(lib::Node(lib::NodeType::STORAGE, i)) - .getState().oneOf(storage_node_up_states())) - { - up = true; - } - } - - if (up) { + if (at_least_one_storage_node_is_available()) { std::vector<document::BucketId> bucketsToCheckForSplit; OperationTargetResolverImpl targetResolver(_bucketSpace, _bucketSpace.getBucketDatabase(), @@ -145,8 +146,8 @@ PutOperation::onStart(DistributorStripeMessageSender& sender) _msg->getBucket().getBucketSpace()); OperationTargetList targets(targetResolver.getTargets(OperationTargetResolver::PUT, bid)); - for (size_t i = 0; i < targets.size(); ++i) { - if (_op_ctx.has_pending_message(targets[i].getNode().getIndex(), targets[i].getBucket(), + for (const auto& target : targets) { + if (_op_ctx.has_pending_message(target.getNode().getIndex(), target.getBucket(), api::MessageType::DELETEBUCKET_ID)) { _tracker.fail(sender, api::ReturnCode(api::ReturnCode::BUCKET_DELETED, @@ -179,13 +180,12 @@ PutOperation::onStart(DistributorStripeMessageSender& sender) std::vector<PersistenceMessageTracker::ToSend> putBatch; // Now send PUTs - for (uint32_t i = 0; i < targets.size(); i++) { - const OperationTarget& target(targets[i]); + for (const auto& target : targets) { sendPutToBucketOnNode(_msg->getBucket().getBucketSpace(), target.getBucketId(), target.getNode().getIndex(), putBatch); } - if (putBatch.size()) { + if (!putBatch.empty()) { _tracker.queueMessageBatch(putBatch); } else { const char* error = "Can't store document: No storage nodes available"; @@ -196,9 +196,9 @@ PutOperation::onStart(DistributorStripeMessageSender& sender) // Check whether buckets are large enough to be split. // TODO(vekterli): only check entries for sendToExisting? - for (uint32_t i = 0; i < entries.size(); ++i) { + for (const auto& entry : entries) { _op_ctx.send_inline_split_if_bucket_too_large(_msg->getBucket().getBucketSpace(), - entries[i], _msg->getPriority()); + entry, _msg->getPriority()); } _tracker.flushQueue(sender); @@ -235,3 +235,5 @@ PutOperation::onClose(DistributorStripeMessageSender& sender) LOG(debug, "%s", error); _tracker.fail(sender, api::ReturnCode(api::ReturnCode::ABORTED, error)); } + +} diff --git a/storage/src/vespa/storage/distributor/operations/external/putoperation.h b/storage/src/vespa/storage/distributor/operations/external/putoperation.h index 9801fed0c99..283395f1406 100644 --- a/storage/src/vespa/storage/distributor/operations/external/putoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/putoperation.h @@ -47,9 +47,10 @@ private: void sendPutToBucketOnNode(document::BucketSpace bucketSpace, const document::BucketId& bucketId, const uint16_t node, std::vector<PersistenceMessageTracker::ToSend>& putBatch); - bool shouldImplicitlyActivateReplica(const OperationTargetList& targets) const; + [[nodiscard]] bool shouldImplicitlyActivateReplica(const OperationTargetList& targets) const; - bool has_unavailable_targets_in_pending_state(const OperationTargetList& targets) const; + [[nodiscard]] bool has_unavailable_targets_in_pending_state(const OperationTargetList& targets) const; + [[nodiscard]] bool at_least_one_storage_node_is_available() const; std::shared_ptr<api::PutCommand> _msg; DistributorStripeOperationContext& _op_ctx; diff --git a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp index 584ad9de2ce..b752a7fadde 100644 --- a/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/removeoperation.cpp @@ -5,13 +5,12 @@ #include <vespa/vdslib/state/clusterstate.h> #include <vespa/log/log.h> -LOG_SETUP(".distributor.operation.external.remove"); +LOG_SETUP(".distributor.operations.external.remove"); - -using namespace storage::distributor; -using namespace storage; using document::BucketSpace; +namespace storage::distributor { + RemoveOperation::RemoveOperation(const DistributorNodeContext& node_ctx, DistributorStripeOperationContext& op_ctx, DistributorBucketSpace& bucketSpace, @@ -100,3 +99,5 @@ RemoveOperation::onClose(DistributorStripeMessageSender& sender) { _tracker.fail(sender, api::ReturnCode(api::ReturnCode::ABORTED, "Process is shutting down")); } + +} |