diff options
Diffstat (limited to 'storage')
15 files changed, 64 insertions, 240 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")); } + +} diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index 9ae6aaf0653..e7eb7a752fb 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -8,11 +8,9 @@ #include <vespa/document/fieldvalue/document.h> #include <vespa/storage/common/bucket_resolver.h> #include <vespa/storageapi/message/datagram.h> -#include <vespa/storageapi/message/documentsummary.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/queryresult.h> #include <vespa/storageapi/message/removelocation.h> -#include <vespa/storageapi/message/searchresult.h> #include <vespa/storageapi/message/stat.h> #include <vespa/storageapi/message/visitor.h> #include <vespa/messagebus/error.h> @@ -237,24 +235,12 @@ DocumentApiConverter::toDocumentAPI(api::StorageCommand& fromMsg) toMsg = std::move(to); break; } - case api::MessageType::SEARCHRESULT_ID: - { - auto & from(static_cast<api::SearchResultCommand&>(fromMsg)); - toMsg = std::make_unique<documentapi::SearchResultMessage>(std::move(from)); - break; - } case api::MessageType::QUERYRESULT_ID: { auto & from(static_cast<api::QueryResultCommand&>(fromMsg)); toMsg = std::make_unique<documentapi::QueryResultMessage>(std::move(from.getSearchResult()), from.getDocumentSummary()); break; } - case api::MessageType::DOCUMENTSUMMARY_ID: - { - auto & from(static_cast<api::DocumentSummaryCommand&>(fromMsg)); - toMsg = std::make_unique<documentapi::DocumentSummaryMessage>(from); - break; - } case api::MessageType::MAPVISITOR_ID: { auto & from(static_cast<api::MapVisitorCommand&>(fromMsg)); diff --git a/storage/src/vespa/storageapi/message/CMakeLists.txt b/storage/src/vespa/storageapi/message/CMakeLists.txt index 2a761921dff..2728b5b51ad 100644 --- a/storage/src/vespa/storageapi/message/CMakeLists.txt +++ b/storage/src/vespa/storageapi/message/CMakeLists.txt @@ -6,9 +6,7 @@ vespa_add_library(storageapi_message OBJECT bucket.cpp visitor.cpp state.cpp - searchresult.cpp bucketsplitting.cpp - documentsummary.cpp stat.cpp removelocation.cpp queryresult.cpp diff --git a/storage/src/vespa/storageapi/message/documentsummary.cpp b/storage/src/vespa/storageapi/message/documentsummary.cpp deleted file mode 100644 index 6909b4d223c..00000000000 --- a/storage/src/vespa/storageapi/message/documentsummary.cpp +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "documentsummary.h" -#include <ostream> - -namespace storage { -namespace api { - -IMPLEMENT_COMMAND(DocumentSummaryCommand, DocumentSummaryReply) -IMPLEMENT_REPLY(DocumentSummaryReply) - -DocumentSummaryCommand::DocumentSummaryCommand() - : StorageCommand(MessageType::DOCUMENTSUMMARY), - DocumentSummary() -{ } - -void -DocumentSummaryCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - out << "DocumentSummary(" << getSummaryCount() << " summaries)"; - if (verbose) { - out << " : "; - StorageCommand::print(out, verbose, indent); - } -} - -DocumentSummaryReply::DocumentSummaryReply(const DocumentSummaryCommand& cmd) - : StorageReply(cmd) -{ } - -void -DocumentSummaryReply::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - out << "DocumentSummaryReply()"; - if (verbose) { - out << " : "; - StorageReply::print(out, verbose, indent); - } -} - -} // api -} // storage diff --git a/storage/src/vespa/storageapi/message/documentsummary.h b/storage/src/vespa/storageapi/message/documentsummary.h deleted file mode 100644 index 5e2c1af3cfd..00000000000 --- a/storage/src/vespa/storageapi/message/documentsummary.h +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "visitor.h" -#include <vespa/vdslib/container/documentsummary.h> - -namespace storage { -namespace api { - -/** - * @class DocumentSummaryCommand - * @ingroup message - * - * @brief The result of a searchvisitor. - */ -class DocumentSummaryCommand : public StorageCommand, - public vdslib::DocumentSummary -{ -public: - explicit DocumentSummaryCommand(); - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - DECLARE_STORAGECOMMAND(DocumentSummaryCommand, onDocumentSummary) -}; - -/** - * @class DocumentSummaryReply - * @ingroup message - * - * @brief Response to a document summary command. - */ -class DocumentSummaryReply : public StorageReply { -public: - explicit DocumentSummaryReply(const DocumentSummaryCommand& command); - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - DECLARE_STORAGEREPLY(DocumentSummaryReply, onDocumentSummaryReply) -}; - -} // api -} // storage diff --git a/storage/src/vespa/storageapi/message/searchresult.cpp b/storage/src/vespa/storageapi/message/searchresult.cpp deleted file mode 100644 index b2cf04b0410..00000000000 --- a/storage/src/vespa/storageapi/message/searchresult.cpp +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "searchresult.h" -#include <ostream> - -using vdslib::SearchResult; - -namespace storage { -namespace api { - -IMPLEMENT_COMMAND(SearchResultCommand, SearchResultReply) -IMPLEMENT_REPLY(SearchResultReply) - -SearchResultCommand::SearchResultCommand() - : StorageCommand(MessageType::SEARCHRESULT), - SearchResult() -{ -} - -void -SearchResultCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - out << "SearchResultCommand(" << getHitCount() << " hits)"; - if (verbose) { - out << " : "; - StorageCommand::print(out, verbose, indent); - } -} - -SearchResultReply::SearchResultReply(const SearchResultCommand& cmd) - : StorageReply(cmd) -{ } - -void -SearchResultReply::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - out << "SearchResultReply()"; - if (verbose) { - out << " : "; - StorageReply::print(out, verbose, indent); - } -} - -} // api -} // storage diff --git a/storage/src/vespa/storageapi/message/searchresult.h b/storage/src/vespa/storageapi/message/searchresult.h deleted file mode 100644 index b12fa5e1613..00000000000 --- a/storage/src/vespa/storageapi/message/searchresult.h +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "visitor.h" -#include <vespa/vdslib/container/searchresult.h> - -namespace storage::api { - -/** - * @class SearchResultCommand - * @ingroup message - * - * @brief The result of a searchvisitor. - */ -class SearchResultCommand : public StorageCommand, public vdslib::SearchResult { -public: - SearchResultCommand(); - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - DECLARE_STORAGECOMMAND(SearchResultCommand, onSearchResult) -}; - -/** - * @class SearchResultReply - * @ingroup message - * - * @brief Response to a search result command. - */ -class SearchResultReply : public StorageReply { -public: - explicit SearchResultReply(const SearchResultCommand& command); - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - DECLARE_STORAGEREPLY(SearchResultReply, onSearchResultReply) -}; - -} diff --git a/storage/src/vespa/storageapi/messageapi/messagehandler.h b/storage/src/vespa/storageapi/messageapi/messagehandler.h index 9ba8542e9db..fa362d5380f 100644 --- a/storage/src/vespa/storageapi/messageapi/messagehandler.h +++ b/storage/src/vespa/storageapi/messageapi/messagehandler.h @@ -29,8 +29,6 @@ class CreateVisitorCommand; // Create a new visitor class DestroyVisitorCommand; // Destroy a running visitor class VisitorInfoCommand; // Sends visitor info to visitor controller class MapVisitorCommand; -class SearchResultCommand; -class DocumentSummaryCommand; class QueryResultCommand; class InternalCommand; @@ -67,8 +65,6 @@ class CreateVisitorReply; class DestroyVisitorReply; class VisitorInfoReply; class MapVisitorReply; -class SearchResultReply; -class DocumentSummaryReply; class QueryResultReply; class InternalReply; @@ -137,12 +133,8 @@ public: virtual bool onVisitorInfoReply(const std::shared_ptr<api::VisitorInfoReply>&) { return false; } virtual bool onMapVisitor(const std::shared_ptr<api::MapVisitorCommand>&) { return false; } virtual bool onMapVisitorReply(const std::shared_ptr<api::MapVisitorReply>&) { return false; } - virtual bool onSearchResult(const std::shared_ptr<api::SearchResultCommand>&) { return false; } - virtual bool onSearchResultReply(const std::shared_ptr<api::SearchResultReply>&) { return false; } virtual bool onQueryResult(const std::shared_ptr<api::QueryResultCommand>&) { return false; } virtual bool onQueryResultReply(const std::shared_ptr<api::QueryResultReply>&) { return false; } - virtual bool onDocumentSummary(const std::shared_ptr<api::DocumentSummaryCommand>&) { return false; } - virtual bool onDocumentSummaryReply(const std::shared_ptr<api::DocumentSummaryReply>&) { return false; } virtual bool onEmptyBuckets(const std::shared_ptr<api::EmptyBucketsCommand>&) { return false; } virtual bool onEmptyBucketsReply(const std::shared_ptr<api::EmptyBucketsReply>&) { return false; } virtual bool onInternal(const std::shared_ptr<api::InternalCommand>&) { return false; } diff --git a/storage/src/vespa/storageapi/messageapi/storagemessage.cpp b/storage/src/vespa/storageapi/messageapi/storagemessage.cpp index b1d68fd77e3..c72ece80476 100644 --- a/storage/src/vespa/storageapi/messageapi/storagemessage.cpp +++ b/storage/src/vespa/storageapi/messageapi/storagemessage.cpp @@ -76,10 +76,6 @@ const MessageType MessageType::APPLYBUCKETDIFF("ApplyBucketDiff", APPLYBUCKETDIF const MessageType MessageType::APPLYBUCKETDIFF_REPLY("ApplyBucketDiff reply", APPLYBUCKETDIFF_REPLY_ID, &MessageType::APPLYBUCKETDIFF); const MessageType MessageType::VISITOR_INFO("VisitorInfo", VISITOR_INFO_ID); const MessageType MessageType::VISITOR_INFO_REPLY("VisitorInfo reply", VISITOR_INFO_REPLY_ID, &MessageType::VISITOR_INFO); -const MessageType MessageType::SEARCHRESULT("SearchResult", SEARCHRESULT_ID); -const MessageType MessageType::SEARCHRESULT_REPLY("SearchResult reply", SEARCHRESULT_REPLY_ID, &MessageType::SEARCHRESULT); -const MessageType MessageType::DOCUMENTSUMMARY("DocumentSummary", DOCUMENTSUMMARY_ID); -const MessageType MessageType::DOCUMENTSUMMARY_REPLY("DocumentSummary reply", DOCUMENTSUMMARY_REPLY_ID, &MessageType::DOCUMENTSUMMARY); const MessageType MessageType::MAPVISITOR("Mapvisitor", MAPVISITOR_ID); const MessageType MessageType::MAPVISITOR_REPLY("Mapvisitor reply", MAPVISITOR_REPLY_ID, &MessageType::MAPVISITOR); const MessageType MessageType::SPLITBUCKET("SplitBucket", SPLITBUCKET_ID); diff --git a/storage/src/vespa/storageapi/messageapi/storagemessage.h b/storage/src/vespa/storageapi/messageapi/storagemessage.h index 282f110646d..4649781c1e5 100644 --- a/storage/src/vespa/storageapi/messageapi/storagemessage.h +++ b/storage/src/vespa/storageapi/messageapi/storagemessage.h @@ -122,14 +122,14 @@ public: DOCBLOCK_REPLY_ID = 59, VISITOR_INFO_ID = 60, VISITOR_INFO_REPLY_ID = 61, - SEARCHRESULT_ID = 64, - SEARCHRESULT_REPLY_ID = 65, + // SEARCHRESULT_ID = 64, + // SEARCHRESULT_REPLY_ID = 65, SPLITBUCKET_ID = 66, SPLITBUCKET_REPLY_ID = 67, JOINBUCKETS_ID = 68, JOINBUCKETS_REPLY_ID = 69, - DOCUMENTSUMMARY_ID = 72, - DOCUMENTSUMMARY_REPLY_ID = 73, + // DOCUMENTSUMMARY_ID = 72, + // DOCUMENTSUMMARY_REPLY_ID = 73, MAPVISITOR_ID = 74, MAPVISITOR_REPLY_ID = 75, STATBUCKET_ID = 76, @@ -208,14 +208,10 @@ public: static const MessageType APPLYBUCKETDIFF_REPLY; static const MessageType VISITOR_INFO; static const MessageType VISITOR_INFO_REPLY; - static const MessageType SEARCHRESULT; - static const MessageType SEARCHRESULT_REPLY; static const MessageType SPLITBUCKET; static const MessageType SPLITBUCKET_REPLY; static const MessageType JOINBUCKETS; static const MessageType JOINBUCKETS_REPLY; - static const MessageType DOCUMENTSUMMARY; - static const MessageType DOCUMENTSUMMARY_REPLY; static const MessageType MAPVISITOR; static const MessageType MAPVISITOR_REPLY; static const MessageType STATBUCKET; |