diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-11-25 18:17:27 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2020-11-26 00:23:06 +0000 |
commit | ee2baa1b001a282bd57318a3f0b8881cdcbc3049 (patch) | |
tree | e1bd88266adb509a9ce4006f7d68cbc59db3c295 | |
parent | e1584673531bc771fa94731da337ce311b4ff7d1 (diff) |
As we have have now removed the expensive Route member we can further compact the message objects.
- Compact StorageMessageAddress to 16 bytes by
- using reference to cluster name.
- Use small enums for protocol and node type.
- Avoid having StorageMessage as separate allocation.
- Avoid default values
43 files changed, 348 insertions, 447 deletions
diff --git a/searchcore/src/apps/vespa-feed-bm/bm_cluster_controller.cpp b/searchcore/src/apps/vespa-feed-bm/bm_cluster_controller.cpp index 15fbb2e2344..18fb0f21fd9 100644 --- a/searchcore/src/apps/vespa-feed-bm/bm_cluster_controller.cpp +++ b/searchcore/src/apps/vespa-feed-bm/bm_cluster_controller.cpp @@ -42,7 +42,8 @@ BmClusterController::BmClusterController(SharedRpcResources& shared_rpc_resource void BmClusterController::set_cluster_up(bool distributor) { - StorageMessageAddress storage_address("storage", distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0); + static vespalib::string _storage; + StorageMessageAddress storage_address(&_storage, distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0); auto req = make_set_cluster_state_request(); auto target_resolver = std::make_unique<storage::rpc::CachingRpcTargetResolver>(_shared_rpc_resources.slobrok_mirror(), _shared_rpc_resources.target_factory(), 1); diff --git a/searchcore/src/apps/vespa-feed-bm/document_api_message_bus_bm_feed_handler.cpp b/searchcore/src/apps/vespa-feed-bm/document_api_message_bus_bm_feed_handler.cpp index 714add169fd..a1429a1c572 100644 --- a/searchcore/src/apps/vespa-feed-bm/document_api_message_bus_bm_feed_handler.cpp +++ b/searchcore/src/apps/vespa-feed-bm/document_api_message_bus_bm_feed_handler.cpp @@ -19,10 +19,14 @@ using storage::lib::NodeType; namespace feedbm { +namespace { + vespalib::string _Storage("storage"); +} + DocumentApiMessageBusBmFeedHandler::DocumentApiMessageBusBmFeedHandler(BmMessageBus &message_bus) : IBmFeedHandler(), _name(vespalib::string("DocumentApiMessageBusBmFeedHandler(distributor)")), - _storage_address(std::make_unique<StorageMessageAddress>("storage", NodeType::DISTRIBUTOR, 0)), + _storage_address(std::make_unique<StorageMessageAddress>(&_Storage, NodeType::DISTRIBUTOR, 0)), _message_bus(message_bus), _route(_storage_address->to_mbus_route()) { diff --git a/searchcore/src/apps/vespa-feed-bm/storage_api_message_bus_bm_feed_handler.cpp b/searchcore/src/apps/vespa-feed-bm/storage_api_message_bus_bm_feed_handler.cpp index 18c1b979895..dcf91ff1901 100644 --- a/searchcore/src/apps/vespa-feed-bm/storage_api_message_bus_bm_feed_handler.cpp +++ b/searchcore/src/apps/vespa-feed-bm/storage_api_message_bus_bm_feed_handler.cpp @@ -17,11 +17,14 @@ using storage::lib::NodeType; namespace feedbm { +namespace { + vespalib::string _Storage("storage"); +} StorageApiMessageBusBmFeedHandler::StorageApiMessageBusBmFeedHandler(BmMessageBus &message_bus, bool distributor) : IBmFeedHandler(), _name(vespalib::string("StorageApiMessageBusBmFeedHandler(") + (distributor ? "distributor" : "service-layer") + ")"), _distributor(distributor), - _storage_address(std::make_unique<StorageMessageAddress>("storage", distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0)), + _storage_address(std::make_unique<StorageMessageAddress>(&_Storage, distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0)), _message_bus(message_bus), _route(_storage_address->to_mbus_route()) { diff --git a/searchcore/src/apps/vespa-feed-bm/storage_api_rpc_bm_feed_handler.cpp b/searchcore/src/apps/vespa-feed-bm/storage_api_rpc_bm_feed_handler.cpp index 55de3e6048f..575cde3fe00 100644 --- a/searchcore/src/apps/vespa-feed-bm/storage_api_rpc_bm_feed_handler.cpp +++ b/searchcore/src/apps/vespa-feed-bm/storage_api_rpc_bm_feed_handler.cpp @@ -26,6 +26,10 @@ using storage::lib::NodeType; namespace feedbm { +namespace { + vespalib::string _Storage("storage"); +} + class StorageApiRpcBmFeedHandler::MyMessageDispatcher : public storage::MessageDispatcher, public StorageReplyErrorChecker { @@ -68,7 +72,7 @@ StorageApiRpcBmFeedHandler::StorageApiRpcBmFeedHandler(SharedRpcResources& share : IBmFeedHandler(), _name(vespalib::string("StorageApiRpcBmFeedHandler(") + (distributor ? "distributor" : "service-layer") + ")"), _distributor(distributor), - _storage_address(std::make_unique<StorageMessageAddress>("storage", distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0)), + _storage_address(std::make_unique<StorageMessageAddress>(&_Storage, distributor ? NodeType::DISTRIBUTOR : NodeType::STORAGE, 0)), _shared_rpc_resources(shared_rpc_resources_in), _message_dispatcher(std::make_unique<MyMessageDispatcher>()), _message_codec_provider(std::make_unique<storage::rpc::MessageCodecProvider>(repo, std::make_shared<documentapi::LoadTypeSet>())), diff --git a/storage/src/tests/distributor/bucketdbupdatertest.cpp b/storage/src/tests/distributor/bucketdbupdatertest.cpp index fa540669b4b..531af27c7f3 100644 --- a/storage/src/tests/distributor/bucketdbupdatertest.cpp +++ b/storage/src/tests/distributor/bucketdbupdatertest.cpp @@ -392,7 +392,8 @@ public: } api::StorageMessageAddress storageAddress(uint16_t node) { - return api::StorageMessageAddress("storage", lib::NodeType::STORAGE, node); + static vespalib::string _storage; + return api::StorageMessageAddress(&_storage, lib::NodeType::STORAGE, node); } std::string getSentNodes(const std::string& oldClusterState, diff --git a/storage/src/tests/distributor/bucketstateoperationtest.cpp b/storage/src/tests/distributor/bucketstateoperationtest.cpp index c62d0a62ed3..60c17eb6a17 100644 --- a/storage/src/tests/distributor/bucketstateoperationtest.cpp +++ b/storage/src/tests/distributor/bucketstateoperationtest.cpp @@ -11,6 +11,10 @@ using namespace ::testing; namespace storage::distributor { +namespace { + vespalib::string _Storage("storage"); +} + struct BucketStateOperationTest : Test, DistributorTestUtil { void SetUp() override { createLinks(); @@ -48,8 +52,7 @@ TEST_F(BucketStateOperationTest, activate_single_node) { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SETBUCKETSTATE); - EXPECT_EQ(api::StorageMessageAddress( - "storage", lib::NodeType::STORAGE, 0).toString(), + EXPECT_EQ(api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 0).toString(), msg->getAddress()->toString()); auto& cmd = dynamic_cast<const api::SetBucketStateCommand&>(*msg); @@ -85,8 +88,7 @@ TEST_F(BucketStateOperationTest, activate_and_deactivate_nodes) { { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SETBUCKETSTATE); - EXPECT_EQ(api::StorageMessageAddress( - "storage", lib::NodeType::STORAGE, 1).toString(), + EXPECT_EQ(api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 1).toString(), msg->getAddress()->toString()); auto& cmd = dynamic_cast<const api::SetBucketStateCommand&>(*msg); @@ -101,8 +103,7 @@ TEST_F(BucketStateOperationTest, activate_and_deactivate_nodes) { { std::shared_ptr<api::StorageCommand> msg = _sender.command(1); ASSERT_EQ(msg->getType(), api::MessageType::SETBUCKETSTATE); - EXPECT_EQ(api::StorageMessageAddress( - "storage", lib::NodeType::STORAGE, 0).toString(), + EXPECT_EQ(api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 0).toString(), msg->getAddress()->toString()); auto& cmd = dynamic_cast<const api::SetBucketStateCommand&>(*msg); @@ -142,8 +143,7 @@ TEST_F(BucketStateOperationTest, do_not_deactivate_if_activate_fails) { { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SETBUCKETSTATE); - EXPECT_EQ(api::StorageMessageAddress( - "storage", lib::NodeType::STORAGE, 1).toString(), + EXPECT_EQ(api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 1).toString(), msg->getAddress()->toString()); auto& cmd = dynamic_cast<const api::SetBucketStateCommand&>(*msg); @@ -185,8 +185,7 @@ TEST_F(BucketStateOperationTest, bucket_db_not_updated_on_failure) { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SETBUCKETSTATE); - EXPECT_EQ(api::StorageMessageAddress( - "storage", lib::NodeType::STORAGE, 0).toString(), + EXPECT_EQ(api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 0).toString(), msg->getAddress()->toString()); std::shared_ptr<api::StorageReply> reply(msg->makeReply().release()); diff --git a/storage/src/tests/distributor/distributor_message_sender_stub.h b/storage/src/tests/distributor/distributor_message_sender_stub.h index 440dee70d48..e69673a9366 100644 --- a/storage/src/tests/distributor/distributor_message_sender_stub.h +++ b/storage/src/tests/distributor/distributor_message_sender_stub.h @@ -11,7 +11,7 @@ namespace storage { class DistributorMessageSenderStub : public distributor::DistributorMessageSender { MessageSenderStub _stub_impl; - std::string _cluster_name; + vespalib::string _cluster_name; distributor::PendingMessageTracker* _pending_message_tracker; public: @@ -82,7 +82,7 @@ public: return 0; } - const std::string& getClusterName() const override { + const vespalib::string& getClusterName() const override { return _cluster_name; } diff --git a/storage/src/tests/distributor/idealstatemanagertest.cpp b/storage/src/tests/distributor/idealstatemanagertest.cpp index fc26a8c9cce..d66b1315fde 100644 --- a/storage/src/tests/distributor/idealstatemanagertest.cpp +++ b/storage/src/tests/distributor/idealstatemanagertest.cpp @@ -56,6 +56,9 @@ struct IdealStateManagerTest : Test, DistributorTestUtil { std::vector<document::BucketSpace> _bucketSpaces; std::string makeBucketStatusString(const std::string &defaultSpaceBucketStatus); }; +namespace { + vespalib::string _Storage("storage"); +} TEST_F(IdealStateManagerTest, sibling) { EXPECT_EQ(document::BucketId(1,1), @@ -170,6 +173,7 @@ TEST_F(IdealStateManagerTest, recheck_when_active) { } TEST_F(IdealStateManagerTest, block_ideal_state_ops_on_full_request_bucket_info) { + setupDistributor(2, 10, "distributor:1 storage:2"); framework::defaultimplementation::FakeClock clock; @@ -182,7 +186,7 @@ TEST_F(IdealStateManagerTest, block_ideal_state_ops_on_full_request_bucket_info) // sent to the entire node. It will then use a null bucketid. { auto msg = std::make_shared<api::RequestBucketInfoCommand>(makeBucketSpace(), buckets); - msg->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 4)); + msg->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 4)); tracker.insert(msg); } @@ -202,7 +206,7 @@ TEST_F(IdealStateManagerTest, block_ideal_state_ops_on_full_request_bucket_info) // Don't block on null-bucket messages that aren't RequestBucketInfo. { auto msg = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "foo", "bar", "baz"); - msg->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 7)); + msg->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 7)); tracker.insert(msg); } @@ -221,8 +225,7 @@ TEST_F(IdealStateManagerTest, block_check_for_all_operations_to_specific_bucket) { auto msg = std::make_shared<api::JoinBucketsCommand>(makeDocumentBucket(bid)); - msg->setAddress( - api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 4)); + msg->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 4)); tracker.insert(msg); } { diff --git a/storage/src/tests/distributor/mergeoperationtest.cpp b/storage/src/tests/distributor/mergeoperationtest.cpp index 75faddbe667..ccd70309a88 100644 --- a/storage/src/tests/distributor/mergeoperationtest.cpp +++ b/storage/src/tests/distributor/mergeoperationtest.cpp @@ -250,7 +250,8 @@ TEST_F(MergeOperationTest, do_not_remove_copies_with_pending_messages) { // at will. auto msg = std::make_shared<api::SetBucketStateCommand>( makeDocumentBucket(bucket), api::SetBucketStateCommand::ACTIVE); - msg->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 1)); + vespalib::string storage("storage"); + msg->setAddress(api::StorageMessageAddress::create(&storage, lib::NodeType::STORAGE, 1)); _pendingTracker->insert(msg); sendReply(op); diff --git a/storage/src/tests/distributor/pendingmessagetrackertest.cpp b/storage/src/tests/distributor/pendingmessagetrackertest.cpp index e1bca1a1890..90171db97d1 100644 --- a/storage/src/tests/distributor/pendingmessagetrackertest.cpp +++ b/storage/src/tests/distributor/pendingmessagetrackertest.cpp @@ -49,6 +49,12 @@ public: std::chrono::milliseconds atTime() const { return _atTime; } }; +api::StorageMessageAddress +makeStorageAddress(uint16_t node) { + static vespalib::string _storage("storage"); + return {&_storage, lib::NodeType::STORAGE, node}; +} + class Fixture { StorageComponentRegisterImpl _compReg; @@ -87,15 +93,9 @@ private: return id.str(); } - document::Document::SP createDummyDocumentForBucket( - const document::BucketId& bucket) const + document::Document::SP createDummyDocumentForBucket(const document::BucketId& bucket) const { - return _testDocMan.createDocument("foobar", - createDummyIdString(bucket)); - } - - api::StorageMessageAddress makeStorageAddress(uint16_t node) const { - return {"storage", lib::NodeType::STORAGE, node}; + return _testDocMan.createDocument("foobar", createDummyIdString(bucket)); } std::shared_ptr<api::PutCommand> createPutToNode(uint16_t node) const { @@ -151,7 +151,7 @@ TEST_F(PendingMessageTrackerTest, simple) { auto remove = std::make_shared<api::RemoveCommand>( makeDocumentBucket(document::BucketId(16, 1234)), document::DocumentId("id:footype:testdoc:n=1234:foo"), 1001); - remove->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0)); + remove->setAddress(makeStorageAddress(0)); tracker.insert(remove); { @@ -186,7 +186,7 @@ PendingMessageTrackerTest::insertMessages(PendingMessageTracker& tracker) auto remove = std::make_shared<api::RemoveCommand>( makeDocumentBucket(document::BucketId(16, 1234)), document::DocumentId(ost.str()), 1000 + i); - remove->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, i % 2)); + remove->setAddress(makeStorageAddress(i % 2)); tracker.insert(remove); } @@ -194,7 +194,7 @@ PendingMessageTrackerTest::insertMessages(PendingMessageTracker& tracker) std::ostringstream ost; ost << "id:footype:testdoc:n=4567:" << i; auto remove = std::make_shared<api::RemoveCommand>(makeDocumentBucket(document::BucketId(16, 4567)), document::DocumentId(ost.str()), 2000 + i); - remove->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, i % 2)); + remove->setAddress(makeStorageAddress(i % 2)); tracker.insert(remove); } } @@ -323,7 +323,7 @@ TEST_F(PendingMessageTrackerTest, get_pending_message_types) { auto remove = std::make_shared<api::RemoveCommand>(makeDocumentBucket(bid), document::DocumentId("id:footype:testdoc:n=1234:foo"), 1001); - remove->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0)); + remove->setAddress(makeStorageAddress(0)); tracker.insert(remove); { @@ -358,7 +358,7 @@ TEST_F(PendingMessageTrackerTest, has_pending_message) { { auto remove = std::make_shared<api::RemoveCommand>(makeDocumentBucket(bid), document::DocumentId("id:footype:testdoc:n=1234:foo"), 1001); - remove->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 1)); + remove->setAddress(makeStorageAddress(1)); tracker.insert(remove); } diff --git a/storage/src/tests/distributor/splitbuckettest.cpp b/storage/src/tests/distributor/splitbuckettest.cpp index d88b02b332e..c876f3e7ee4 100644 --- a/storage/src/tests/distributor/splitbuckettest.cpp +++ b/storage/src/tests/distributor/splitbuckettest.cpp @@ -44,6 +44,11 @@ SplitOperationTest::SplitOperationTest() { } +namespace { + vespalib::string _Storage("storage"); + api::StorageMessageAddress _Storage0Address(&_Storage, lib::NodeType::STORAGE, 0); +} + TEST_F(SplitOperationTest, simple) { enableDistributorClusterState("distributor:1 storage:1"); @@ -65,7 +70,7 @@ TEST_F(SplitOperationTest, simple) { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SPLITBUCKET); - EXPECT_EQ(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0).toString(), + EXPECT_EQ(_Storage0Address.toString(), msg->getAddress()->toString()); std::shared_ptr<api::StorageReply> reply(msg->makeReply().release()); @@ -135,7 +140,7 @@ TEST_F(SplitOperationTest, multi_node_failure) { { std::shared_ptr<api::StorageCommand> msg = _sender.command(0); ASSERT_EQ(msg->getType(), api::MessageType::SPLITBUCKET); - EXPECT_EQ(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0).toString(), + EXPECT_EQ(_Storage0Address.toString(), msg->getAddress()->toString()); auto* sreply = static_cast<api::SplitBucketReply*>(msg->makeReply().release()); @@ -264,8 +269,7 @@ TEST_F(SplitOperationTest, operation_blocked_by_pending_join) { }; auto joinCmd = std::make_shared<api::JoinBucketsCommand>(makeDocumentBucket(joinTarget)); joinCmd->getSourceBuckets() = joinSources; - joinCmd->setAddress( - api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0)); + joinCmd->setAddress(_Storage0Address); tracker.insert(joinCmd); @@ -284,8 +288,7 @@ TEST_F(SplitOperationTest, operation_blocked_by_pending_join) { tracker.clearMessagesForNode(0); EXPECT_FALSE(op.isBlocked(tracker)); - joinCmd->setAddress( - api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 1)); + joinCmd->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); tracker.insert(joinCmd); EXPECT_TRUE(op.isBlocked(tracker)); diff --git a/storage/src/tests/persistence/common/filestortestfixture.cpp b/storage/src/tests/persistence/common/filestortestfixture.cpp index 079abd6df06..57c63747ece 100644 --- a/storage/src/tests/persistence/common/filestortestfixture.cpp +++ b/storage/src/tests/persistence/common/filestortestfixture.cpp @@ -83,9 +83,11 @@ FileStorTestFixture::TestFileStorComponents::TestFileStorComponents( top.open(); } +vespalib::string _Storage("storage"); + api::StorageMessageAddress FileStorTestFixture::makeSelfAddress() { - return api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0); + return api::StorageMessageAddress(&_Storage, lib::NodeType::STORAGE, 0); } void diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index 3cac188cb17..709981921a6 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -57,6 +57,11 @@ namespace storage { namespace { metrics::LoadType defaultLoadType(0, "default"); +vespalib::string _Cluster("cluster"); +vespalib::string _Storage("storage"); +api::StorageMessageAddress _Storage2(&_Storage, lib::NodeType::STORAGE, 2); +api::StorageMessageAddress _Storage3(&_Storage, lib::NodeType::STORAGE, 3); +api::StorageMessageAddress _Cluster1(&_Cluster, lib::NodeType::STORAGE, 1); struct TestFileStorComponents; @@ -322,7 +327,6 @@ struct FileStorManagerTest : public FileStorTestBase { TEST_F(FileStorManagerTest, header_only_put) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); // Creating a document to test with Document::SP doc(createDocument( "some content", "id:crawler:testdoctype1:n=4000:foo").release()); @@ -334,7 +338,7 @@ TEST_F(FileStorManagerTest, header_only_put) { // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 105); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -349,7 +353,7 @@ TEST_F(FileStorManagerTest, header_only_put) { { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 124); cmd->setUpdateTimestamp(105); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -361,7 +365,7 @@ TEST_F(FileStorManagerTest, header_only_put) { // Getting it { auto cmd = std::make_shared<api::GetCommand>(makeDocumentBucket(bid), doc->getId(), document::AllFields::NAME); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -385,7 +389,6 @@ TEST_F(FileStorManagerTest, header_only_put) { TEST_F(FileStorManagerTest, put) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); // Creating a document to test with Document::SP doc(createDocument( "some content", "id:crawler:testdoctype1:n=4000:foo").release()); @@ -397,7 +400,7 @@ TEST_F(FileStorManagerTest, put) { // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 105); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -422,7 +425,6 @@ TEST_F(FileStorManagerTest, state_change) { TEST_F(FileStorManagerTest, flush) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); // Creating a document to test with document::DocumentId docId("id:ns:testdoctype1::crawler:http://www.ntnu.no/"); @@ -435,7 +437,7 @@ TEST_F(FileStorManagerTest, flush) { std::vector<std::shared_ptr<api::StorageCommand> > _commands; for (uint32_t i=0; i<msgCount; ++i) { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, i+1); - cmd->setAddress(address); + cmd->setAddress(_Storage3); _commands.push_back(cmd); } for (uint32_t i=0; i<msgCount; ++i) { @@ -462,8 +464,7 @@ TEST_F(FileStorManagerTest, handler_priority) { // Populate bucket with the given data for (uint32_t i = 1; i < 6; i++) { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), doc, 100); - auto address = std::make_shared<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setPriority(i * 15); filestorHandler.schedule(cmd); } @@ -588,8 +589,7 @@ TEST_F(FileStorManagerTest, handler_pause) { // Populate bucket with the given data for (uint32_t i = 1; i < 6; i++) { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), doc, 100); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setPriority(i * 15); filestorHandler.schedule(cmd); } @@ -665,8 +665,7 @@ TEST_F(FileStorManagerTest, handler_timeout) { // Populate bucket with the given data { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), doc, 100); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setPriority(0); cmd->setTimeout(50ms); filestorHandler.schedule(cmd); @@ -674,8 +673,7 @@ TEST_F(FileStorManagerTest, handler_timeout) { { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), doc, 100); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setPriority(200); cmd->setTimeout(10000ms); filestorHandler.schedule(cmd); @@ -738,8 +736,7 @@ TEST_F(FileStorManagerTest, priority) { document::BucketId bucket(16, factory.getBucketId(documents[i]->getId()).getRawId()); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), documents[i], 100 + i); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setPriority(i * 2); filestorHandler.schedule(cmd); } @@ -755,9 +752,7 @@ TEST_F(FileStorManagerTest, priority) { ASSERT_LT(count, 10000); for (uint32_t i = 0; i < documents.size(); i++) { - std::shared_ptr<api::PutReply> reply( - std::dynamic_pointer_cast<api::PutReply>( - c.top.getReply(i))); + std::shared_ptr<api::PutReply> reply(std::dynamic_pointer_cast<api::PutReply>(c.top.getReply(i))); ASSERT_TRUE(reply.get()); EXPECT_EQ(ReturnCode(ReturnCode::OK), reply->getResult()); } @@ -799,8 +794,7 @@ TEST_F(FileStorManagerTest, split1) { _node->getPersistenceProvider().createBucket(makeSpiBucket(bucket), context); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), documents[i], 100 + i); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); cmd->setSourceIndex(0); filestorHandler.schedule(cmd); @@ -814,9 +808,8 @@ TEST_F(FileStorManagerTest, split1) { // Delete every 5th document to have delete entries in file too if (i % 5 == 0) { - auto rcmd = std::make_shared<api::RemoveCommand>( - makeDocumentBucket(bucket), documents[i]->getId(), 1000000 + 100 + i); - rcmd->setAddress(*address); + auto rcmd = std::make_shared<api::RemoveCommand>(makeDocumentBucket(bucket), documents[i]->getId(), 1000000 + 100 + i); + rcmd->setAddress(_Storage3); filestorHandler.schedule(rcmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -842,12 +835,9 @@ TEST_F(FileStorManagerTest, split1) { // Test that the documents have gotten into correct parts. for (uint32_t i=0; i<documents.size(); ++i) { - document::BucketId bucket( - 17, i % 3 == 0 ? 0x10001 : 0x0100001); - auto cmd = std::make_shared<api::GetCommand>( - makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(address); + document::BucketId bucket(17, i % 3 == 0 ? 0x10001 : 0x0100001); + auto cmd = std::make_shared<api::GetCommand>(makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -859,8 +849,7 @@ TEST_F(FileStorManagerTest, split1) { // Keep splitting location 1 until we gidsplit for (int i=17; i<=32; ++i) { - auto cmd = std::make_shared<api::SplitBucketCommand>( - makeDocumentBucket(document::BucketId(i, 0x0100001))); + auto cmd = std::make_shared<api::SplitBucketCommand>(makeDocumentBucket(document::BucketId(i, 0x0100001))); cmd->setSourceIndex(0); filestorHandler.schedule(cmd); filestorHandler.flush(true); @@ -877,13 +866,10 @@ TEST_F(FileStorManagerTest, split1) { if (i % 3 == 0) { bucket = document::BucketId(17, 0x10001); } else { - bucket = document::BucketId(33, factory.getBucketId( - documents[i]->getId()).getRawId()); + bucket = document::BucketId(33, factory.getBucketId(documents[i]->getId()).getRawId()); } - auto cmd = std::make_shared<api::GetCommand>( - makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(address); + auto cmd = std::make_shared<api::GetCommand>(makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -932,8 +918,7 @@ TEST_F(FileStorManagerTest, split_single_group) { _node->getPersistenceProvider().createBucket(makeSpiBucket(bucket), context); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), documents[i], 100 + i); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(address); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -958,10 +943,8 @@ TEST_F(FileStorManagerTest, split_single_group) { // Test that the documents are all still there for (uint32_t i=0; i<documents.size(); ++i) { document::BucketId bucket(17, state ? 0x10001 : 0x00001); - auto cmd = std::make_shared<api::GetCommand> - (makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(address); + auto cmd = std::make_shared<api::GetCommand>(makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -982,7 +965,6 @@ FileStorTestBase::putDoc(DummyStorageLink& top, const document::BucketId& target, uint32_t docNum) { - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); spi::Context context(spi::Priority(0), spi::Trace::TraceLevel(0)); document::BucketIdFactory factory; document::DocumentId docId(vespalib::make_string("id:ns:testdoctype1:n=%" PRIu64 ":%d", target.getId(), docNum)); @@ -991,7 +973,7 @@ FileStorTestBase::putDoc(DummyStorageLink& top, _node->getPersistenceProvider().createBucket(makeSpiBucket(target), context); Document::SP doc(new Document(*_testdoctype1, docId)); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(target), doc, docNum+1); - cmd->setAddress(address); + cmd->setAddress(_Storage3); cmd->setPriority(120); filestorHandler.schedule(cmd); filestorHandler.flush(true); @@ -1012,8 +994,6 @@ TEST_F(FileStorManagerTest, split_empty_target_with_remapped_ops) { document::BucketId source(16, 0x10001); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - for (uint32_t i=0; i<10; ++i) { ASSERT_NO_FATAL_FAILURE(putDoc(top, filestorHandler, source, i)); } @@ -1034,7 +1014,7 @@ TEST_F(FileStorManagerTest, split_empty_target_with_remapped_ops) { vespalib::make_string("id:ns:testdoctype1:n=%d:1234", 0x100001)); auto doc = std::make_shared<Document>(*_testdoctype1, docId); auto putCmd = std::make_shared<api::PutCommand>(makeDocumentBucket(source), doc, 1001); - putCmd->setAddress(address); + putCmd->setAddress(_Storage3); putCmd->setPriority(120); filestorHandler.schedule(splitCmd); @@ -1115,8 +1095,7 @@ TEST_F(FileStorManagerTest, join) { for (uint32_t i=0; i<documents.size(); ++i) { document::BucketId bucket(17, factory.getBucketId(documents[i]->getId()).getRawId()); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), documents[i], 100 + i); - auto address = std::make_unique<api::StorageMessageAddress>("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(*address); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -1128,7 +1107,7 @@ TEST_F(FileStorManagerTest, join) { if ((i % 5) == 0) { auto rcmd = std::make_shared<api::RemoveCommand>( makeDocumentBucket(bucket), documents[i]->getId(), 1000000 + 100 + i); - rcmd->setAddress(*address); + rcmd->setAddress(_Storage3); filestorHandler.schedule(rcmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -1157,8 +1136,7 @@ TEST_F(FileStorManagerTest, join) { document::BucketId bucket(16, 1); auto cmd = std::make_shared<api::GetCommand>( makeDocumentBucket(bucket), documents[i]->getId(), document::AllFields::NAME); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - cmd->setAddress(address); + cmd->setAddress(_Storage3); filestorHandler.schedule(cmd); filestorHandler.flush(true); ASSERT_EQ(1, top.getNumReplies()); @@ -1320,7 +1298,6 @@ TEST_F(FileStorManagerTest, visiting) { TEST_F(FileStorManagerTest, remove_location) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); document::BucketId bid(8, 0); createBucket(bid); @@ -1331,7 +1308,7 @@ TEST_F(FileStorManagerTest, remove_location) { docid << "id:ns:testdoctype1:n=" << (i << 8) << ":foo"; Document::SP doc(createDocument("some content", docid.str())); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 1000 + i); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1344,7 +1321,7 @@ TEST_F(FileStorManagerTest, remove_location) { // Issuing remove location command { auto cmd = std::make_shared<api::RemoveLocationCommand>("id.user % 512 == 0", makeDocumentBucket(bid)); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1359,7 +1336,6 @@ TEST_F(FileStorManagerTest, remove_location) { TEST_F(FileStorManagerTest, delete_bucket) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 2); // Creating a document to test with document::DocumentId docId("id:crawler:testdoctype1:n=4000:http://www.ntnu.no/"); auto doc = std::make_shared<Document>(*_testdoctype1, docId); @@ -1371,7 +1347,7 @@ TEST_F(FileStorManagerTest, delete_bucket) { // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 105); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1387,7 +1363,7 @@ TEST_F(FileStorManagerTest, delete_bucket) { // Delete bucket { auto cmd = std::make_shared<api::DeleteBucketCommand>(makeDocumentBucket(bid)); - cmd->setAddress(address); + cmd->setAddress(_Storage3); cmd->setBucketInfo(bucketInfo); top.sendDown(cmd); top.waitForMessages(1, _waitTime); @@ -1401,7 +1377,6 @@ TEST_F(FileStorManagerTest, delete_bucket) { TEST_F(FileStorManagerTest, delete_bucket_rejects_outdated_bucket_info) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 2); // Creating a document to test with document::DocumentId docId("id:crawler:testdoctype1:n=4000:http://www.ntnu.no/"); Document::SP doc(new Document(*_testdoctype1, docId)); @@ -1414,7 +1389,7 @@ TEST_F(FileStorManagerTest, delete_bucket_rejects_outdated_bucket_info) { // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 105); - cmd->setAddress(address); + cmd->setAddress(_Storage2); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1431,7 +1406,7 @@ TEST_F(FileStorManagerTest, delete_bucket_rejects_outdated_bucket_info) { { auto cmd = std::make_shared<api::DeleteBucketCommand>(makeDocumentBucket(bid)); cmd->setBucketInfo(api::BucketInfo(0xf000baaa, 1, 123, 1, 456)); - cmd->setAddress(address); + cmd->setAddress(_Storage2); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1449,7 +1424,6 @@ TEST_F(FileStorManagerTest, delete_bucket_rejects_outdated_bucket_info) { TEST_F(FileStorManagerTest, delete_bucket_with_invalid_bucket_info){ TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 2); // Creating a document to test with document::DocumentId docId("id:crawler:testdoctype1:n=4000:http://www.ntnu.no/"); auto doc = std::make_shared<Document>(*_testdoctype1, docId); @@ -1460,7 +1434,7 @@ TEST_F(FileStorManagerTest, delete_bucket_with_invalid_bucket_info){ // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 105); - cmd->setAddress(address); + cmd->setAddress(_Storage2); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1474,7 +1448,7 @@ TEST_F(FileStorManagerTest, delete_bucket_with_invalid_bucket_info){ // Attempt to delete bucket with invalid bucketinfo { auto cmd = std::make_shared<api::DeleteBucketCommand>(makeDocumentBucket(bid)); - cmd->setAddress(address); + cmd->setAddress(_Storage2); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1488,8 +1462,6 @@ TEST_F(FileStorManagerTest, delete_bucket_with_invalid_bucket_info){ TEST_F(FileStorManagerTest, no_timestamps) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address( - "storage", lib::NodeType::STORAGE, 3); // Creating a document to test with Document::SP doc(createDocument( "some content", "id:ns:testdoctype1::crawler:http://www.ntnu.no/").release()); @@ -1500,7 +1472,7 @@ TEST_F(FileStorManagerTest, no_timestamps) { // Putting it { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 0); - cmd->setAddress(address); + cmd->setAddress(_Storage3); EXPECT_EQ(api::Timestamp(0), cmd->getTimestamp()); top.sendDown(cmd); top.waitForMessages(1, _waitTime); @@ -1513,7 +1485,7 @@ TEST_F(FileStorManagerTest, no_timestamps) { // Removing it { auto cmd = std::make_shared<api::RemoveCommand>(makeDocumentBucket(bid), doc->getId(), 0); - cmd->setAddress(address); + cmd->setAddress(_Storage3); EXPECT_EQ(api::Timestamp(0), cmd->getTimestamp()); top.sendDown(cmd); top.waitForMessages(1, _waitTime); @@ -1528,7 +1500,6 @@ TEST_F(FileStorManagerTest, no_timestamps) { TEST_F(FileStorManagerTest, equal_timestamps) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); // Creating a document to test with document::BucketId bid(16, 4000); @@ -1539,7 +1510,7 @@ TEST_F(FileStorManagerTest, equal_timestamps) { Document::SP doc(createDocument( "some content", "id:crawler:testdoctype1:n=4000:http://www.ntnu.no/")); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 100); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1556,7 +1527,7 @@ TEST_F(FileStorManagerTest, equal_timestamps) { Document::SP doc(createDocument( "some content", "id:crawler:testdoctype1:n=4000:http://www.ntnu.no/")); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 100); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1571,7 +1542,7 @@ TEST_F(FileStorManagerTest, equal_timestamps) { Document::SP doc(createDocument( "some content", "id:crawler:testdoctype1:n=4000:http://www.ntnu.nu/")); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), doc, 100); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1585,8 +1556,6 @@ TEST_F(FileStorManagerTest, equal_timestamps) { TEST_F(FileStorManagerTest, get_iter) { TestFileStorComponents c(*this); auto& top = c.top; - api::StorageMessageAddress address( - "storage", lib::NodeType::STORAGE, 3); document::BucketId bid(16, 4000); createBucket(bid); @@ -1605,7 +1574,7 @@ TEST_F(FileStorManagerTest, get_iter) { // Putting all docs to have something to visit for (uint32_t i=0; i<docs.size(); ++i) { auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), docs[i], 100 + i); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1659,7 +1628,6 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { TestFileStorComponents c(*this); auto& top = c.top; setClusterState("storage:4 distributor:1"); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); document::BucketId bid(16, 4000); @@ -1668,9 +1636,8 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { EXPECT_FALSE(provider.isActive(makeSpiBucket(bid))); { - auto cmd = std::make_shared<api::SetBucketStateCommand>( - makeDocumentBucket(bid), api::SetBucketStateCommand::ACTIVE); - cmd->setAddress(address); + auto cmd = std::make_shared<api::SetBucketStateCommand>(makeDocumentBucket(bid), api::SetBucketStateCommand::ACTIVE); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1682,9 +1649,7 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { EXPECT_TRUE(provider.isActive(makeSpiBucket(bid))); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); EXPECT_TRUE(entry->info.isActive()); } // Trigger bucket info to be read back into the database @@ -1699,16 +1664,14 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { } // Should not have lost active flag { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); EXPECT_TRUE(entry->info.isActive()); } { auto cmd = std::make_shared<api::SetBucketStateCommand>( makeDocumentBucket(bid), api::SetBucketStateCommand::INACTIVE); - cmd->setAddress(address); + cmd->setAddress(_Storage3); top.sendDown(cmd); top.waitForMessages(1, _waitTime); ASSERT_EQ(1, top.getNumReplies()); @@ -1720,9 +1683,7 @@ TEST_F(FileStorManagerTest, set_bucket_active_state) { EXPECT_FALSE(provider.isActive(makeSpiBucket(bid))); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); EXPECT_FALSE(entry->info.isActive()); } } @@ -1736,9 +1697,8 @@ TEST_F(FileStorManagerTest, notify_owner_distributor_on_outdated_set_bucket_stat ASSERT_NE(bid.getRawId(), 0); createBucket(bid); - auto cmd = std::make_shared<api::SetBucketStateCommand>( - makeDocumentBucket(bid), api::SetBucketStateCommand::ACTIVE); - cmd->setAddress(api::StorageMessageAddress("cluster", lib::NodeType::STORAGE, 1)); + auto cmd = std::make_shared<api::SetBucketStateCommand>(makeDocumentBucket(bid), api::SetBucketStateCommand::ACTIVE); + cmd->setAddress(_Cluster1); cmd->setSourceIndex(0); top.sendDown(cmd); @@ -1773,7 +1733,7 @@ TEST_F(FileStorManagerTest, GetBucketDiff_implicitly_creates_bucket) { std::vector<api::MergeBucketCommand::Node> nodes = {1, 0}; auto cmd = std::make_shared<api::GetBucketDiffCommand>(makeDocumentBucket(bid), nodes, Timestamp(1000)); - cmd->setAddress(api::StorageMessageAddress("cluster", lib::NodeType::STORAGE, 1)); + cmd->setAddress(_Cluster1); cmd->setSourceIndex(0); top.sendDown(cmd); @@ -1781,9 +1741,7 @@ TEST_F(FileStorManagerTest, GetBucketDiff_implicitly_creates_bucket) { ASSERT_SINGLE_REPLY(api::GetBucketDiffReply, reply, top, _waitTime); EXPECT_EQ(api::ReturnCode(api::ReturnCode::OK), reply->getResult()); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); ASSERT_TRUE(entry.exist()); EXPECT_TRUE(entry->info.isReady()); } @@ -1799,16 +1757,14 @@ TEST_F(FileStorManagerTest, merge_bucket_implicitly_creates_bucket) { std::vector<api::MergeBucketCommand::Node> nodes = {1, 2}; auto cmd = std::make_shared<api::MergeBucketCommand>(makeDocumentBucket(bid), nodes, Timestamp(1000)); - cmd->setAddress(api::StorageMessageAddress("cluster", lib::NodeType::STORAGE, 1)); + cmd->setAddress(_Cluster1); cmd->setSourceIndex(0); top.sendDown(cmd); api::GetBucketDiffCommand* diffCmd; ASSERT_SINGLE_REPLY(api::GetBucketDiffCommand, diffCmd, top, _waitTime); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); ASSERT_TRUE(entry.exist()); EXPECT_TRUE(entry->info.isReady()); } @@ -1822,7 +1778,7 @@ TEST_F(FileStorManagerTest, newly_created_bucket_is_ready) { document::BucketId bid(16, 4000); auto cmd = std::make_shared<api::CreateBucketCommand>(makeDocumentBucket(bid)); - cmd->setAddress(api::StorageMessageAddress("cluster", lib::NodeType::STORAGE, 1)); + cmd->setAddress(_Cluster1); cmd->setSourceIndex(0); top.sendDown(cmd); @@ -1830,9 +1786,7 @@ TEST_F(FileStorManagerTest, newly_created_bucket_is_ready) { ASSERT_SINGLE_REPLY(api::CreateBucketReply, reply, top, _waitTime); EXPECT_EQ(api::ReturnCode(api::ReturnCode::OK), reply->getResult()); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); ASSERT_TRUE(entry.exist()); EXPECT_TRUE(entry->info.isReady()); EXPECT_FALSE(entry->info.isActive()); @@ -1844,10 +1798,8 @@ TEST_F(FileStorManagerTest, create_bucket_sets_active_flag_in_database_and_reply setClusterState("storage:2 distributor:1"); document::BucketId bid(16, 4000); - std::shared_ptr<api::CreateBucketCommand> cmd( - new api::CreateBucketCommand(makeDocumentBucket(bid))); - cmd->setAddress(api::StorageMessageAddress( - "cluster", lib::NodeType::STORAGE, 1)); + auto cmd = std::make_shared<api::CreateBucketCommand>(makeDocumentBucket(bid)); + cmd->setAddress(_Cluster1); cmd->setSourceIndex(0); cmd->setActive(true); c.top.sendDown(cmd); @@ -1856,9 +1808,7 @@ TEST_F(FileStorManagerTest, create_bucket_sets_active_flag_in_database_and_reply ASSERT_SINGLE_REPLY(api::CreateBucketReply, reply, c.top, _waitTime); EXPECT_EQ(api::ReturnCode(api::ReturnCode::OK), reply->getResult()); { - StorBucketDatabase::WrappedEntry entry( - _node->getStorageBucketDatabase().get( - bid, "foo")); + StorBucketDatabase::WrappedEntry entry(_node->getStorageBucketDatabase().get(bid, "foo")); ASSERT_TRUE(entry.exist()); EXPECT_TRUE(entry->info.isReady()); EXPECT_TRUE(entry->info.isActive()); @@ -1867,9 +1817,8 @@ TEST_F(FileStorManagerTest, create_bucket_sets_active_flag_in_database_and_reply template <typename Metric> void FileStorTestBase::assert_request_size_set(TestFileStorComponents& c, std::shared_ptr<api::StorageMessage> cmd, const Metric& metric) { - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); cmd->setApproxByteSize(54321); - cmd->setAddress(address); + cmd->setAddress(_Storage3); c.top.sendDown(cmd); c.top.waitForMessages(1, _waitTime); EXPECT_EQ(static_cast<int64_t>(cmd->getApproxByteSize()), metric.request_size.getLast()); @@ -1926,7 +1875,7 @@ TEST_F(FileStorManagerTest, test_and_set_condition_mismatch_not_counted_as_failu createBucket(bucket); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bucket), std::move(doc), api::Timestamp(12345)); cmd->setCondition(TestAndSetCondition("not testdoctype1")); - cmd->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 3)); + cmd->setAddress(_Storage3); c.top.sendDown(cmd); api::PutReply* reply; diff --git a/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp b/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp index 8b38083b33d..2710d766f5d 100644 --- a/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp +++ b/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp @@ -32,7 +32,8 @@ namespace { api::StorageMessageAddress makeAddress() { - return api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 0); + static vespalib::string _storage("storage"); + return api::StorageMessageAddress(&_storage, lib::NodeType::STORAGE, 0); } void diff --git a/storage/src/tests/storageserver/communicationmanagertest.cpp b/storage/src/tests/storageserver/communicationmanagertest.cpp index 1431c49559c..b352c06d1d0 100644 --- a/storage/src/tests/storageserver/communicationmanagertest.cpp +++ b/storage/src/tests/storageserver/communicationmanagertest.cpp @@ -24,6 +24,8 @@ using namespace ::testing; namespace storage { +vespalib::string _Storage("storage"); + struct CommunicationManagerTest : Test { static constexpr uint32_t MESSAGE_WAIT_TIME_SEC = 60; @@ -36,7 +38,7 @@ struct CommunicationManagerTest : Test { auto cmd = std::make_shared<api::GetCommand>(makeDocumentBucket(document::BucketId(0)), document::DocumentId("id:ns:mytype::mydoc"), document::AllFields::NAME); - cmd->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 1)); + cmd->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); cmd->setPriority(priority); return cmd; } @@ -72,7 +74,7 @@ TEST_F(CommunicationManagerTest, simple) { // Send a message through from distributor to storage auto cmd = std::make_shared<api::GetCommand>( makeDocumentBucket(document::BucketId(0)), document::DocumentId("id:ns:mytype::mydoc"), document::AllFields::NAME); - cmd->setAddress(api::StorageMessageAddress("storage", lib::NodeType::STORAGE, 1)); + cmd->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); distributorLink->sendUp(cmd); storageLink->waitForMessages(1, MESSAGE_WAIT_TIME_SEC); ASSERT_GT(storageLink->getNumCommands(), 0); diff --git a/storage/src/tests/storageserver/mergethrottlertest.cpp b/storage/src/tests/storageserver/mergethrottlertest.cpp index 0271af85fef..d8156b21333 100644 --- a/storage/src/tests/storageserver/mergethrottlertest.cpp +++ b/storage/src/tests/storageserver/mergethrottlertest.cpp @@ -29,6 +29,8 @@ namespace storage { namespace { +vespalib::string _Storage("storage"); + struct MergeBuilder { document::BucketId _bucket; api::Timestamp _maxTimestamp; @@ -104,8 +106,7 @@ struct MergeBuilder { auto cmd = std::make_shared<MergeBucketCommand>( makeDocumentBucket(_bucket), n, _maxTimestamp, _clusterStateVersion, _chain); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, _nodes[0]); - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, _nodes[0])); return cmd; } }; @@ -115,8 +116,7 @@ MergeBuilder::~MergeBuilder() = default; std::shared_ptr<api::SetSystemStateCommand> makeSystemStateCmd(const std::string& state) { - return std::make_shared<api::SetSystemStateCommand>( - lib::ClusterState(state)); + return std::make_shared<api::SetSystemStateCommand>(lib::ClusterState(state)); } } // anon ns @@ -266,8 +266,7 @@ TEST_F(MergeThrottlerTest, chain) { auto cmd = std::make_shared<MergeBucketCommand>(bucket, nodes, UINT_MAX, 123); cmd->setPriority(7); cmd->setTimeout(54321ms); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 0)); const uint16_t distributorIndex = 123; cmd->setSourceIndex(distributorIndex); // Dummy distributor index that must be forwarded @@ -404,15 +403,13 @@ TEST_F(MergeThrottlerTest, chain) { TEST_F(MergeThrottlerTest, with_source_only_node) { BucketId bid(14, 0x1337); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); - std::vector<MergeBucketCommand::Node> nodes; nodes.push_back(0); nodes.push_back(2); nodes.push_back(MergeBucketCommand::Node(1, true)); auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, UINT_MAX, 123); - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 0)); _topLinks[0]->sendDown(cmd); _topLinks[0]->waitForMessage(MessageType::MERGEBUCKET, _messageWaitTime); @@ -460,9 +457,7 @@ TEST_F(MergeThrottlerTest, legacy_42_distributor_behavior) { auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, 1234); // Send to node 1, which is not the lowest index - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 1); - - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); _topLinks[1]->sendDown(cmd); _bottomLinks[1]->waitForMessage(MessageType::MERGEBUCKET, _messageWaitTime); @@ -500,9 +495,7 @@ TEST_F(MergeThrottlerTest, legacy_42_distributor_behavior_does_not_take_ownershi auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, 1234); // Send to node 1, which is not the lowest index - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 1); - - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); _topLinks[1]->sendDown(cmd); _bottomLinks[1]->waitForMessage(MessageType::MERGEBUCKET, _messageWaitTime); @@ -554,9 +547,7 @@ TEST_F(MergeThrottlerTest, end_of_chain_execution_does_not_take_ownership) { auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, 1234, 1, chain); // Send to last node, which is not the lowest index - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 3); - - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 3)); _topLinks[2]->sendDown(cmd); _bottomLinks[2]->waitForMessage(MessageType::MERGEBUCKET, _messageWaitTime); @@ -604,9 +595,7 @@ TEST_F(MergeThrottlerTest, resend_handling) { nodes.push_back(2); auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, 1234); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 1); - - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); _topLinks[0]->sendDown(cmd); _topLinks[0]->waitForMessage(MessageType::MERGEBUCKET, _messageWaitTime); @@ -1008,9 +997,8 @@ TEST_F(MergeThrottlerTest, unseen_merge_with_node_in_chain) { auto cmd = std::make_shared<MergeBucketCommand>( makeDocumentBucket(BucketId(32, 0xdeadbeef)), nodes, 1234, 1, chain); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 9); - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 9)); _topLinks[0]->sendDown(cmd); // First, test that we get rejected when processing merge immediately @@ -1218,9 +1206,7 @@ TEST_F(MergeThrottlerTest, unknown_merge_with_self_in_chain) { chain.push_back(0); auto cmd = std::make_shared<MergeBucketCommand>(makeDocumentBucket(bid), nodes, 1234, 1, chain); - StorageMessageAddress address("storage", lib::NodeType::STORAGE, 1); - - cmd->setAddress(address); + cmd->setAddress(StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); _topLinks[0]->sendDown(cmd); _topLinks[0]->waitForMessage(MessageType::MERGEBUCKET_REPLY, _messageWaitTime); diff --git a/storage/src/tests/storageserver/rpc/caching_rpc_target_resolver_test.cpp b/storage/src/tests/storageserver/rpc/caching_rpc_target_resolver_test.cpp index 6291a8ad0dd..8f8eb84b0f2 100644 --- a/storage/src/tests/storageserver/rpc/caching_rpc_target_resolver_test.cpp +++ b/storage/src/tests/storageserver/rpc/caching_rpc_target_resolver_test.cpp @@ -49,6 +49,8 @@ public: } }; +vespalib::string _my_cluster("my_cluster"); + class CachingRpcTargetResolverTest : public ::testing::Test { public: MockMirror mirror; @@ -66,8 +68,8 @@ public: : mirror(), factory(), resolver(mirror, factory, 2), - address_0("my_cluster", NodeType::STORAGE, 5), - address_1("my_cluster", NodeType::DISTRIBUTOR, 7), + address_0(&_my_cluster, NodeType::STORAGE, 5), + address_1(&_my_cluster, NodeType::DISTRIBUTOR, 7), spec_0("tcp/my:41"), spec_1("tcp/my:42"), bucket_id_0(3), diff --git a/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp b/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp index 69d5a827272..7f2279e1f4e 100644 --- a/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp +++ b/storage/src/tests/storageserver/rpc/storage_api_rpc_service_test.cpp @@ -90,7 +90,8 @@ LockingMockOperationDispatcher::LockingMockOperationDispatcher() = default; LockingMockOperationDispatcher::~LockingMockOperationDispatcher() = default; api::StorageMessageAddress make_address(uint16_t node_index, bool is_distributor) { - return {"coolcluster", (is_distributor ? lib::NodeType::DISTRIBUTOR : lib::NodeType::STORAGE), node_index}; + static vespalib::string _coolcluster("coolcluster"); + return {&_coolcluster, (is_distributor ? lib::NodeType::DISTRIBUTOR : lib::NodeType::STORAGE), node_index}; } vespalib::string to_slobrok_id(const api::StorageMessageAddress& address) { diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp index 7943790f13d..6b222d3c4ed 100644 --- a/storage/src/tests/visiting/visitormanagertest.cpp +++ b/storage/src/tests/visiting/visitormanagertest.cpp @@ -36,7 +36,8 @@ namespace storage { namespace { using msg_ptr_vector = std::vector<api::StorageMessage::SP>; - +vespalib::string _Storage; +api::StorageMessageAddress _Address(&_Storage, lib::NodeType::STORAGE, 0); } struct VisitorManagerTest : Test { @@ -101,7 +102,6 @@ VisitorManagerTest::initializeTest() _top->open(); // Adding some documents so database isn't empty - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); std::string content( "To be, or not to be: that is the question:\n" "Whether 'tis nobler in the mind to suffer\n" @@ -152,7 +152,7 @@ VisitorManagerTest::initializeTest() document::BucketId bid(16, i); auto cmd = std::make_shared<api::CreateBucketCommand>(makeDocumentBucket(bid)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setSourceIndex(0); _top->sendDown(cmd); _top->waitForMessages(1, 60); @@ -167,7 +167,7 @@ VisitorManagerTest::initializeTest() document::BucketId bid(16, i); auto cmd = std::make_shared<api::PutCommand>(makeDocumentBucket(bid), _documents[i], i+1); - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); _top->waitForMessages(1, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); @@ -182,13 +182,12 @@ void VisitorManagerTest::addSomeRemoves(bool removeAll) { framework::defaultimplementation::FakeClock clock; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); for (uint32_t i=0; i<docCount; i += (removeAll ? 1 : 4)) { // Add it to the database document::BucketId bid(16, i % 10); auto cmd = std::make_shared<api::RemoveCommand>( makeDocumentBucket(bid), _documents[i]->getId(), clock.getTimeInMicros().getTime() + docCount + i + 1); - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); _top->waitForMessages(1, 60); const msg_ptr_vector replies = _top->getRepliesOnce(); @@ -345,10 +344,9 @@ int getTotalSerializedSize(const std::vector<document::Document::SP>& docs) TEST_F(VisitorManagerTest, normal_usage) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setControlDestination("foo/bar"); _top->sendDown(cmd); std::vector<document::Document::SP > docs; @@ -369,10 +367,9 @@ TEST_F(VisitorManagerTest, normal_usage) { TEST_F(VisitorManagerTest, resending) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setControlDestination("foo/bar"); _top->sendDown(cmd); std::vector<document::Document::SP > docs; @@ -416,11 +413,10 @@ TEST_F(VisitorManagerTest, resending) { TEST_F(VisitorManagerTest, visit_empty_bucket) { ASSERT_NO_FATAL_FAILURE(initializeTest()); addSomeRemoves(true); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); // All data has been replied to, expecting to get a create visitor reply @@ -429,12 +425,11 @@ TEST_F(VisitorManagerTest, visit_empty_bucket) { TEST_F(VisitorManagerTest, multi_bucket_visit) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); for (uint32_t i=0; i<10; ++i) { cmd->addBucketToBeVisited(document::BucketId(16, i)); } - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setDataDestination("fooclient.0"); _top->sendDown(cmd); std::vector<document::Document::SP> docs; @@ -451,10 +446,9 @@ TEST_F(VisitorManagerTest, multi_bucket_visit) { TEST_F(VisitorManagerTest, no_buckets) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); // Should get one reply; a CreateVisitorReply with error since no @@ -472,9 +466,8 @@ TEST_F(VisitorManagerTest, no_buckets) { TEST_F(VisitorManagerTest, visit_puts_and_removes) { ASSERT_NO_FATAL_FAILURE(initializeTest()); addSomeRemoves(); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setVisitRemoves(); for (uint32_t i=0; i<10; ++i) { cmd->addBucketToBeVisited(document::BucketId(16, i)); @@ -496,14 +489,13 @@ TEST_F(VisitorManagerTest, visit_puts_and_removes) { TEST_F(VisitorManagerTest, visit_with_timeframe_and_selection) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", "testdoctype1.headerval < 2"); cmd->setFromTime(3); cmd->setToTime(8); for (uint32_t i=0; i<10; ++i) { cmd->addBucketToBeVisited(document::BucketId(16, i)); } - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); std::vector<document::Document::SP> docs; std::vector<document::DocumentId> docIds; @@ -525,7 +517,6 @@ TEST_F(VisitorManagerTest, visit_with_timeframe_and_selection) { TEST_F(VisitorManagerTest, visit_with_timeframe_and_bogus_selection) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", "DocType(testdoctype1---///---) XXX BAD Field(headerval) < 2"); cmd->setFromTime(3); @@ -533,7 +524,7 @@ TEST_F(VisitorManagerTest, visit_with_timeframe_and_bogus_selection) { for (uint32_t i=0; i<10; ++i) { cmd->addBucketToBeVisited(document::BucketId(16, i)); } - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); _top->waitForMessages(1, 60); @@ -566,11 +557,10 @@ TEST_F(VisitorManagerTest, visit_with_timeframe_and_bogus_selection) { TEST_F(VisitorManagerTest, visitor_callbacks) { ASSERT_NO_FATAL_FAILURE(initializeTest()); std::ostringstream replydata; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "TestVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); cmd->addBucketToBeVisited(document::BucketId(16, 5)); - cmd->setAddress(address); + cmd->setAddress(_Address); _top->sendDown(cmd); // Wait until we have started the visitor @@ -607,7 +597,6 @@ TEST_F(VisitorManagerTest, visitor_callbacks) { TEST_F(VisitorManagerTest, visitor_cleanup) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); // Start a bunch of invalid visitors for (uint32_t i=0; i<10; ++i) { @@ -615,7 +604,7 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { ost << "testvis" << i; auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "InvalidVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); _top->sendDown(cmd); _top->waitForMessages(i+1, 60); @@ -627,7 +616,7 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { ost << "testvis" << (i + 10); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); _top->sendDown(cmd); } @@ -696,7 +685,7 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { ost << "testvis" << (i + 24); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); _top->sendDown(cmd); } @@ -723,12 +712,11 @@ TEST_F(VisitorManagerTest, visitor_cleanup) { TEST_F(VisitorManagerTest, abort_on_failed_visitor_info) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); { auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); _top->sendDown(cmd); } @@ -757,13 +745,12 @@ TEST_F(VisitorManagerTest, abort_on_failed_visitor_info) { TEST_F(VisitorManagerTest, abort_on_field_path_error) { initializeTest(); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); // Use bogus field path to force error to happen auto cmd = std::make_shared<api::CreateVisitorCommand>( makeBucketSpace(), "DumpVisitor", "testvis", "testdoctype1.headerval{bogus} == 1234"); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); _top->sendDown(cmd); @@ -772,7 +759,6 @@ TEST_F(VisitorManagerTest, abort_on_field_path_error) { TEST_F(VisitorManagerTest, visitor_queue_timeout) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); _manager->enforceQueueUsage(); { @@ -780,7 +766,7 @@ TEST_F(VisitorManagerTest, visitor_queue_timeout) { auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(1ms); cmd->setTimeout(100 * 1000 * 1000ms); _top->sendDown(cmd); @@ -801,11 +787,10 @@ TEST_F(VisitorManagerTest, visitor_queue_timeout) { TEST_F(VisitorManagerTest, visitor_processing_timeout) { ASSERT_NO_FATAL_FAILURE(initializeTest()); - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(0ms); cmd->setTimeout(100ms); _top->sendDown(cmd); @@ -827,10 +812,9 @@ api::StorageMessage::Id sendCreateVisitor(vespalib::duration timeout, DummyStorageLink& top, uint8_t priority = 127) { std::ostringstream ost; ost << "testvis" << ++nextVisitor; - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>(makeBucketSpace(), "DumpVisitor", ost.str(), ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); - cmd->setAddress(address); + cmd->setAddress(_Address); cmd->setQueueTimeout(timeout); cmd->setPriority(priority); top.sendDown(cmd); diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index f727cdf8eb2..9d0d5575993 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -440,7 +440,8 @@ VisitorTest::fetchSingleCommand(DummyStorageLink& link, std::shared_ptr<T>& msg_ std::shared_ptr<api::CreateVisitorCommand> VisitorTest::makeCreateVisitor(const VisitorOptions& options) { - api::StorageMessageAddress address("storage", lib::NodeType::STORAGE, 0); + static vespalib::string _storage("storage"); + api::StorageMessageAddress address(&_storage, lib::NodeType::STORAGE, 0); auto cmd = std::make_shared<api::CreateVisitorCommand>( makeBucketSpace(), options.visitorType, "testvis", ""); cmd->addBucketToBeVisited(document::BucketId(16, 3)); diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index e4563ec56ce..525add2ede2 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -132,7 +132,7 @@ Distributor::getDistributorIndex() const return _component.getIndex(); } -const std::string& +const vespalib::string& Distributor::getClusterName() const { return _clusterName; diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index d7dd1fda2e9..23c763ce120 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -150,7 +150,7 @@ public: } int getDistributorIndex() const override; - const std::string& getClusterName() const override; + const vespalib::string& getClusterName() const override; const PendingMessageTracker& getPendingMessageTracker() const override; void sendCommand(const std::shared_ptr<api::StorageCommand>&) override; void sendReply(const std::shared_ptr<api::StorageReply>&) override; @@ -319,7 +319,7 @@ private: MaintenanceScheduler::SchedulingMode _schedulingMode; framework::MilliSecTimer _recoveryTimeStarted; framework::ThreadWaitInfo _tickResult; - const std::string _clusterName; + const vespalib::string _clusterName; BucketDBMetricUpdater _bucketDBMetricUpdater; std::unique_ptr<BucketGcTimeCalculator::BucketIdHasher> _bucketIdHasher; MetricUpdateHook _metricUpdateHook; diff --git a/storage/src/vespa/storage/distributor/distributorcomponent.cpp b/storage/src/vespa/storage/distributor/distributorcomponent.cpp index 1c84376dea6..519413bab2e 100644 --- a/storage/src/vespa/storage/distributor/distributorcomponent.cpp +++ b/storage/src/vespa/storage/distributor/distributorcomponent.cpp @@ -132,30 +132,21 @@ DistributorComponent::ownsBucketInCurrentState(const document::Bucket &bucket) c api::StorageMessageAddress DistributorComponent::nodeAddress(uint16_t nodeIndex) const { - return api::StorageMessageAddress( - getClusterName(), - lib::NodeType::STORAGE, - nodeIndex); + return api::StorageMessageAddress::create(&getClusterName(), lib::NodeType::STORAGE, nodeIndex); } bool -DistributorComponent::checkDistribution( - api::StorageCommand &cmd, - const document::Bucket &bucket) +DistributorComponent::checkDistribution(api::StorageCommand &cmd, const document::Bucket &bucket) { BucketOwnership bo(checkOwnershipInPendingAndCurrentState(bucket)); if (!bo.isOwned()) { std::string systemStateStr = bo.getNonOwnedState().toString(); LOG(debug, - "Got message with wrong distribution, " - "bucket %s sending back state '%s'", - bucket.toString().c_str(), - systemStateStr.c_str()); + "Got message with wrong distribution, bucket %s sending back state '%s'", + bucket.toString().c_str(), systemStateStr.c_str()); api::StorageReply::UP reply(cmd.makeReply()); - api::ReturnCode ret( - api::ReturnCode::WRONG_DISTRIBUTION, - systemStateStr); + api::ReturnCode ret(api::ReturnCode::WRONG_DISTRIBUTION, systemStateStr); reply->setResult(ret); sendUp(std::shared_ptr<api::StorageMessage>(reply.release())); return false; @@ -164,8 +155,7 @@ DistributorComponent::checkDistribution( } void -DistributorComponent::removeNodesFromDB(const document::Bucket &bucket, - const std::vector<uint16_t>& nodes) +DistributorComponent::removeNodesFromDB(const document::Bucket &bucket, const std::vector<uint16_t>& nodes) { auto &bucketSpace(_bucketSpaceRepo.get(bucket.getBucketSpace())); BucketDatabase::Entry dbentry = bucketSpace.getBucketDatabase().get(bucket.getBucketId()); diff --git a/storage/src/vespa/storage/distributor/distributormessagesender.cpp b/storage/src/vespa/storage/distributor/distributormessagesender.cpp index d40bd4bd9c2..a9e18d00d8d 100644 --- a/storage/src/vespa/storage/distributor/distributormessagesender.cpp +++ b/storage/src/vespa/storage/distributor/distributormessagesender.cpp @@ -10,10 +10,9 @@ DistributorMessageSender::sendToNode(const lib::NodeType& nodeType, uint16_t nod const std::shared_ptr<api::StorageCommand> & cmd, bool useDocumentAPI) { cmd->setSourceIndex(getDistributorIndex()); - cmd->setAddress(api::StorageMessageAddress(getClusterName(), nodeType, node, - (useDocumentAPI - ? api::StorageMessageAddress::DOCUMENT - : api::StorageMessageAddress::STORAGE))); + cmd->setAddress(useDocumentAPI + ? api::StorageMessageAddress::createDocApi(&getClusterName(), nodeType, node) + : api::StorageMessageAddress::create(&getClusterName(), nodeType, node)); uint64_t msgId = cmd->getMsgId(); sendCommand(cmd); return msgId; diff --git a/storage/src/vespa/storage/distributor/distributormessagesender.h b/storage/src/vespa/storage/distributor/distributormessagesender.h index 078762dd05c..151413e98ef 100644 --- a/storage/src/vespa/storage/distributor/distributormessagesender.h +++ b/storage/src/vespa/storage/distributor/distributormessagesender.h @@ -2,6 +2,7 @@ #pragma once #include <vespa/storage/common/messagesender.h> +#include <vespa/vespalib/stllike/string.h> namespace storage::lib { class NodeType; } namespace storage::distributor { @@ -18,7 +19,7 @@ public: const std::shared_ptr<api::StorageCommand>& cmd, bool useDocumentAPI = false); virtual int getDistributorIndex() const = 0; - virtual const std::string& getClusterName() const = 0; + virtual const vespalib::string& getClusterName() const = 0; virtual const PendingMessageTracker& getPendingMessageTracker() const = 0; }; diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp index 7e8eaaff15e..c580e4aa419 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp @@ -43,7 +43,7 @@ public: int getDistributorIndex() const override { return _distributor.getDistributorIndex(); // Thread safe } - const std::string& getClusterName() const override { + const vespalib::string& getClusterName() const override { return _distributor.getClusterName(); // Thread safe } const PendingMessageTracker& getPendingMessageTracker() const override { diff --git a/storage/src/vespa/storage/distributor/messagetracker.cpp b/storage/src/vespa/storage/distributor/messagetracker.cpp index c20b862597a..298ec028d71 100644 --- a/storage/src/vespa/storage/distributor/messagetracker.cpp +++ b/storage/src/vespa/storage/distributor/messagetracker.cpp @@ -19,8 +19,7 @@ void MessageTracker::flushQueue(MessageSender& sender) { for (uint32_t i = 0; i < _commandQueue.size(); i++) { - _commandQueue[i]._msg->setAddress( - api::StorageMessageAddress(_clusterName, lib::NodeType::STORAGE, _commandQueue[i]._target)); + _commandQueue[i]._msg->setAddress(api::StorageMessageAddress::create(&_clusterName, lib::NodeType::STORAGE, _commandQueue[i]._target)); _sentMessages[_commandQueue[i]._msg->getMsgId()] = _commandQueue[i]._target; sender.sendCommand(_commandQueue[i]._msg); } diff --git a/storage/src/vespa/storage/distributor/messagetracker.h b/storage/src/vespa/storage/distributor/messagetracker.h index 11d8c36082e..44fa86a4f9e 100644 --- a/storage/src/vespa/storage/distributor/messagetracker.h +++ b/storage/src/vespa/storage/distributor/messagetracker.h @@ -2,9 +2,9 @@ #pragma once #include <vespa/storage/common/messagesender.h> +#include <vespa/vespalib/stllike/string.h> #include <vector> #include <map> -#include <string> namespace storage::api { class BucketCommand; @@ -50,7 +50,7 @@ protected: // Keeps track of which node a message was sent to. std::map<uint64_t, uint16_t> _sentMessages; - std::string _clusterName; + vespalib::string _clusterName; }; } diff --git a/storage/src/vespa/storage/distributor/operationowner.h b/storage/src/vespa/storage/distributor/operationowner.h index bcd5a8419f5..39881550466 100644 --- a/storage/src/vespa/storage/distributor/operationowner.h +++ b/storage/src/vespa/storage/distributor/operationowner.h @@ -40,7 +40,7 @@ public: return _sender.getDistributorIndex(); } - const std::string& getClusterName() const override { + const vespalib::string& getClusterName() const override { return _sender.getClusterName(); } diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index d7dd0be7f4f..19427d8037e 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -74,7 +74,7 @@ struct IntermediateMessageSender : DistributorMessageSender { return forward.getDistributorIndex(); } - const std::string& getClusterName() const override { + const vespalib::string& getClusterName() const override { return forward.getClusterName(); } diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 0868f18e88a..18255326115 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -662,9 +662,7 @@ bool VisitorOperation::bucketIsValidAndConsistent(const BucketDatabase::Entry& entry) const { if (!entry.valid()) { - LOG(debug, - "Bucket %s does not exist anymore", - entry.toString().c_str()); + LOG(debug, "Bucket %s does not exist anymore", entry.toString().c_str()); return false; } assert(entry->getNodeCount() != 0); @@ -696,10 +694,8 @@ VisitorOperation::assignBucketsToNodes(NodeToBucketsMap& nodeToBucketsMap) BucketInfo& bucketInfo(subIter->second); if (shouldSkipBucket(bucketInfo)) { - LOG(spam, - "Skipping subbucket %s because it is done/active/failed: %s", - subBucket.toString().c_str(), - bucketInfo.toString().c_str()); + LOG(spam, "Skipping subbucket %s because it is done/active/failed: %s", + subBucket.toString().c_str(), bucketInfo.toString().c_str()); continue; } @@ -721,25 +717,13 @@ VisitorOperation::assignBucketsToNodes(NodeToBucketsMap& nodeToBucketsMap) } int -VisitorOperation::getNumVisitorsToSendForNode(uint16_t node, - uint32_t totalBucketsOnNode) const +VisitorOperation::getNumVisitorsToSendForNode(uint16_t node, uint32_t totalBucketsOnNode) const { - int visitorCountAvailable( - std::max(1, static_cast<int>(_config.maxVisitorsPerNodePerVisitor - - _activeNodes[node]))); - - int visitorCountMinBucketsPerVisitor( - std::max(1, static_cast<int>(totalBucketsOnNode / _config.minBucketsPerVisitor))); - - int visitorCount( - std::min(visitorCountAvailable, visitorCountMinBucketsPerVisitor)); - LOG(spam, - "Will send %d visitors to node %d (available=%d, " - "buckets restricted=%d)", - visitorCount, - node, - visitorCountAvailable, - visitorCountMinBucketsPerVisitor); + int visitorCountAvailable(std::max(1, static_cast<int>(_config.maxVisitorsPerNodePerVisitor - _activeNodes[node]))); + int visitorCountMinBucketsPerVisitor(std::max(1, static_cast<int>(totalBucketsOnNode / _config.minBucketsPerVisitor))); + int visitorCount(std::min(visitorCountAvailable, visitorCountMinBucketsPerVisitor)); + LOG(spam, "Will send %d visitors to node %d (available=%d, buckets restricted=%d)", + visitorCount, node, visitorCountAvailable, visitorCountMinBucketsPerVisitor); return visitorCount; } @@ -749,31 +733,24 @@ VisitorOperation::sendStorageVisitors(const NodeToBucketsMap& nodeToBucketsMap, DistributorMessageSender& sender) { bool visitorsSent = false; - for (NodeToBucketsMap::const_iterator iter = nodeToBucketsMap.begin(); - iter != nodeToBucketsMap.end(); - ++iter) { - if (iter->second.size() > 0) { - int visitorCount(getNumVisitorsToSendForNode(iter->first, iter->second.size())); + for (const auto & entry : nodeToBucketsMap ) { + if (entry.second.size() > 0) { + int visitorCount(getNumVisitorsToSendForNode(entry.first, entry.second.size())); std::vector<std::vector<document::BucketId> > bucketsVector(visitorCount); - for (unsigned int i = 0; i < iter->second.size(); i++) { - bucketsVector[i % visitorCount].push_back(iter->second[i]); + for (unsigned int i = 0; i < entry.second.size(); i++) { + bucketsVector[i % visitorCount].push_back(entry.second[i]); } for (int i = 0; i < visitorCount; i++) { - LOG(spam, - "Send visitor to node %d with %u buckets", - iter->first, - (unsigned int)bucketsVector[i].size()); + LOG(spam, "Send visitor to node %d with %u buckets", + entry.first, (unsigned int)bucketsVector[i].size()); - sendStorageVisitor(iter->first, - bucketsVector[i], - _msg->getMaximumPendingReplyCount(), - sender); + sendStorageVisitor(entry.first, bucketsVector[i], _msg->getMaximumPendingReplyCount(), sender); visitorsSent = true; } } else { - LOG(spam, "Do not send visitor to node %d, no buckets", iter->first); + LOG(spam, "Do not send visitor to node %d, no buckets", entry.first); } } return visitorsSent; @@ -791,7 +768,7 @@ VisitorOperation::sendStorageVisitor(uint16_t node, uint32_t pending, DistributorMessageSender& sender) { - api::CreateVisitorCommand::SP cmd(new api::CreateVisitorCommand(*_msg)); + auto cmd = std::make_shared<api::CreateVisitorCommand>(*_msg); cmd->getBuckets() = buckets; // TODO: Send this through distributor - do after moving visitor stuff from docapi to storageprotocol @@ -804,8 +781,7 @@ VisitorOperation::sendStorageVisitor(uint16_t node, vespalib::string storageInstanceId(os.str()); cmd->setInstanceId(storageInstanceId); - cmd->setAddress(api::StorageMessageAddress(_owner.getClusterName(), - lib::NodeType::STORAGE, node)); + cmd->setAddress(api::StorageMessageAddress::create(&_owner.getClusterName(), lib::NodeType::STORAGE, node)); cmd->setMaximumPendingReplyCount(pending); cmd->setQueueTimeout(computeVisitorQueueTimeoutMs()); @@ -867,8 +843,7 @@ VisitorOperation::updateReplyMetrics(const api::ReturnCode& result) void VisitorOperation::onClose(DistributorMessageSender& sender) { - sendReply(api::ReturnCode(api::ReturnCode::ABORTED, "Process is shutting down"), - sender); + sendReply(api::ReturnCode(api::ReturnCode::ABORTED, "Process is shutting down"), sender); } } diff --git a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp index 76547dc83a8..9a2fe2cd6ce 100644 --- a/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp +++ b/storage/src/vespa/storage/persistence/bucketownershipnotifier.cpp @@ -60,10 +60,7 @@ BucketOwnershipNotifier::sendNotifyBucketToDistributor( } auto notifyCmd = std::make_shared<api::NotifyBucketChangeCommand>(bucket, infoToSend); - notifyCmd->setAddress(api::StorageMessageAddress( - _component.getClusterName(), - lib::NodeType::DISTRIBUTOR, - distributorIndex)); + notifyCmd->setAddress(api::StorageMessageAddress::create(&_component.getClusterName(), lib::NodeType::DISTRIBUTOR, distributorIndex)); notifyCmd->setSourceIndex(_component.getIndex()); LOG(debug, "Sending notify to distributor %u: %s", diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 2e65302ee3b..b3d3b1b2737 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -294,8 +294,8 @@ namespace { return value; } - api::StorageMessageAddress createAddress(const std::string& clusterName, uint16_t node) { - return api::StorageMessageAddress(clusterName, lib::NodeType::STORAGE, node); + api::StorageMessageAddress createAddress(const vespalib::string * clusterName, uint16_t node) { + return api::StorageMessageAddress::create(clusterName, lib::NodeType::STORAGE, node); } void assertContainedInBucket(const document::DocumentId& docId, @@ -706,7 +706,7 @@ MergeHandler::processBucketMerge(const spi::Bucket& bucket, MergeStatus& status, assert(nodes.size() > 1); cmd = std::make_shared<api::ApplyBucketDiffCommand>(bucket.getBucket(), nodes); - cmd->setAddress(createAddress(_clusterName, nodes[1].index)); + cmd->setAddress(createAddress(&_clusterName, nodes[1].index)); findCandidates(status, active_nodes_mask, true, @@ -782,7 +782,7 @@ MergeHandler::processBucketMerge(const spi::Bucket& bucket, MergeStatus& status, } assert(nodes.size() > 1); cmd = std::make_shared<api::ApplyBucketDiffCommand>(bucket.getBucket(), nodes); - cmd->setAddress(createAddress(_clusterName, nodes[1].index)); + cmd->setAddress(createAddress(&_clusterName, nodes[1].index)); // Add all the metadata, and thus use big limit. Max // data to fetch parameter will control amount added. findCandidates(status, active_nodes_mask, true, e.first, newMask, *cmd); @@ -795,7 +795,7 @@ MergeHandler::processBucketMerge(const spi::Bucket& bucket, MergeStatus& status, // merge to merge the remaining data. if ( ! cmd ) { cmd = std::make_shared<api::ApplyBucketDiffCommand>(bucket.getBucket(), status.nodeList); - cmd->setAddress(createAddress(_clusterName, status.nodeList[1].index)); + cmd->setAddress(createAddress(&_clusterName, status.nodeList[1].index)); findCandidates(status, active_nodes_mask, false, 0, 0, *cmd); } cmd->setPriority(status.context.getPriority()); @@ -901,7 +901,7 @@ MergeHandler::handleMergeBucket(api::MergeBucketCommand& cmd, MessageTracker::UP bucket.toString().c_str(), s->nodeList[1].index, uint32_t(cmd2->getDiff().size())); - cmd2->setAddress(createAddress(_clusterName, s->nodeList[1].index)); + cmd2->setAddress(createAddress(&_clusterName, s->nodeList[1].index)); cmd2->setPriority(s->context.getPriority()); cmd2->setTimeout(s->timeout); cmd2->setSourceIndex(cmd.getSourceIndex()); @@ -1111,7 +1111,7 @@ MergeHandler::handleGetBucketDiff(api::GetBucketDiffCommand& cmd, MessageTracker bucket.toString().c_str(), cmd.getNodes()[index + 1].index, local.size() - remote.size()); auto cmd2 = std::make_shared<api::GetBucketDiffCommand>(bucket.getBucket(), cmd.getNodes(), cmd.getMaxTimestamp()); - cmd2->setAddress(createAddress(_clusterName, cmd.getNodes()[index + 1].index)); + cmd2->setAddress(createAddress(&_clusterName, cmd.getNodes()[index + 1].index)); cmd2->getDiff().swap(local); cmd2->setPriority(cmd.getPriority()); cmd2->setTimeout(cmd.getTimeout()); @@ -1293,7 +1293,7 @@ MergeHandler::handleApplyBucketDiff(api::ApplyBucketDiffCommand& cmd, MessageTra LOG(spam, "Sending ApplyBucketDiff for %s on to node %d", bucket.toString().c_str(), cmd.getNodes()[index + 1].index); auto cmd2 = std::make_shared<api::ApplyBucketDiffCommand>(bucket.getBucket(), cmd.getNodes()); - cmd2->setAddress(createAddress(_clusterName, cmd.getNodes()[index + 1].index)); + cmd2->setAddress(createAddress(&_clusterName, cmd.getNodes()[index + 1].index)); cmd2->getDiff().swap(cmd.getDiff()); cmd2->setPriority(cmd.getPriority()); cmd2->setTimeout(cmd.getTimeout()); diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index c296f215c8c..a229b6c1c58 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -562,8 +562,8 @@ CommunicationManager::sendCommand( api::StorageMessageAddress address(*msg->getAddress()); switch (msg->getType().getId()) { case api::MessageType::STATBUCKET_ID: { - if (address.getProtocol() == api::StorageMessageAddress::STORAGE) { - address.setProtocol(api::StorageMessageAddress::DOCUMENT); + if (address.getProtocol() == api::StorageMessageAddress::Protocol::STORAGE) { + address.setProtocol(api::StorageMessageAddress::Protocol::DOCUMENT); } } default: @@ -572,7 +572,7 @@ CommunicationManager::sendCommand( framework::MilliSecTimer startTime(_component.getClock()); switch (address.getProtocol()) { - case api::StorageMessageAddress::STORAGE: + case api::StorageMessageAddress::Protocol::STORAGE: { LOG(debug, "Send to %s: %s", address.toString().c_str(), msg->toString().c_str()); if (_use_direct_storageapi_rpc && _storage_api_rpc_service->target_supports_direct_rpc(address)) { @@ -588,7 +588,7 @@ CommunicationManager::sendCommand( } break; } - case api::StorageMessageAddress::DOCUMENT: + case api::StorageMessageAddress::Protocol::DOCUMENT: { MBUS_TRACE(msg->getTrace(), 7, "Communication manager: Converting storageapi message to documentapi"); diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp index 2e3c65ef70c..fd68b8ef821 100644 --- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp +++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp @@ -374,11 +374,7 @@ MergeThrottler::forwardCommandToNode( mergeCmd.getMaxTimestamp(), mergeCmd.getClusterStateVersion(), newChain)); - fwdMerge->setAddress( - api::StorageMessageAddress( - _component.getClusterName(), - lib::NodeType::STORAGE, - nodeIndex)); + fwdMerge->setAddress(api::StorageMessageAddress::create(&_component.getClusterName(), lib::NodeType::STORAGE, nodeIndex)); fwdMerge->setSourceIndex(mergeCmd.getSourceIndex()); fwdMerge->setPriority(mergeCmd.getPriority()); fwdMerge->setTimeout(mergeCmd.getTimeout()); diff --git a/storage/src/vespa/storage/storageserver/rpc/caching_rpc_target_resolver.cpp b/storage/src/vespa/storage/storageserver/rpc/caching_rpc_target_resolver.cpp index c497421f8f7..39959c24968 100644 --- a/storage/src/vespa/storage/storageserver/rpc/caching_rpc_target_resolver.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/caching_rpc_target_resolver.cpp @@ -29,7 +29,7 @@ vespalib::string CachingRpcTargetResolver::address_to_slobrok_id(const api::StorageMessageAddress& address) { vespalib::asciistream as; as << "storage/cluster." << address.getCluster() - << '/' << ((address.getNodeType() == lib::NodeType::STORAGE) ? "storage" : "distributor") + << '/' << ((address.getNodeType() == lib::NodeType::Type::STORAGE) ? "storage" : "distributor") << '/' << address.getIndex(); return as.str(); } diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index e413a62ae39..e807ab673d5 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -115,7 +115,8 @@ namespace { } TEST_F(StorageProtocolTest, testAddress50) { - StorageMessageAddress address("foo", lib::NodeType::STORAGE, 3); + vespalib::string cluster("foo"); + StorageMessageAddress address(&cluster, lib::NodeType::STORAGE, 3); EXPECT_EQ(vespalib::string("storage/cluster.foo/storage/3/default"), address.to_mbus_route().toString()); } @@ -816,25 +817,25 @@ TEST_P(StorageProtocolTest, serialized_size_is_used_to_set_approx_size_of_storag } TEST_P(StorageProtocolTest, track_memory_footprint_for_some_messages) { - EXPECT_EQ(64u, sizeof(StorageMessage)); - EXPECT_EQ(80u, sizeof(StorageReply)); - EXPECT_EQ(104u, sizeof(BucketReply)); + EXPECT_EQ(72u, sizeof(StorageMessage)); + EXPECT_EQ(88u, sizeof(StorageReply)); + EXPECT_EQ(112u, sizeof(BucketReply)); EXPECT_EQ(8u, sizeof(document::BucketId)); EXPECT_EQ(16u, sizeof(document::Bucket)); EXPECT_EQ(32u, sizeof(BucketInfo)); - EXPECT_EQ(136u, sizeof(BucketInfoReply)); - EXPECT_EQ(280u, sizeof(PutReply)); - EXPECT_EQ(264u, sizeof(UpdateReply)); - EXPECT_EQ(256u, sizeof(RemoveReply)); - EXPECT_EQ(344u, sizeof(GetReply)); - EXPECT_EQ(80u, sizeof(StorageCommand)); - EXPECT_EQ(104u, sizeof(BucketCommand)); - EXPECT_EQ(104u, sizeof(BucketInfoCommand)); - EXPECT_EQ(104u + sizeof(std::string), sizeof(TestAndSetCommand)); - EXPECT_EQ(136u + sizeof(std::string), sizeof(PutCommand)); - EXPECT_EQ(136u + sizeof(std::string), sizeof(UpdateCommand)); - EXPECT_EQ(216u + sizeof(std::string), sizeof(RemoveCommand)); - EXPECT_EQ(288u, sizeof(GetCommand)); + EXPECT_EQ(144u, sizeof(BucketInfoReply)); + EXPECT_EQ(288u, sizeof(PutReply)); + EXPECT_EQ(272u, sizeof(UpdateReply)); + EXPECT_EQ(264u, sizeof(RemoveReply)); + EXPECT_EQ(352u, sizeof(GetReply)); + EXPECT_EQ(88u, sizeof(StorageCommand)); + EXPECT_EQ(112u, sizeof(BucketCommand)); + EXPECT_EQ(112u, sizeof(BucketInfoCommand)); + EXPECT_EQ(112u + sizeof(std::string), sizeof(TestAndSetCommand)); + EXPECT_EQ(144u + sizeof(std::string), sizeof(PutCommand)); + EXPECT_EQ(144u + sizeof(std::string), sizeof(UpdateCommand)); + EXPECT_EQ(224u + sizeof(std::string), sizeof(RemoveCommand)); + EXPECT_EQ(296u, sizeof(GetCommand)); } } // storage::api diff --git a/storageapi/src/tests/messageapi/storage_message_address_test.cpp b/storageapi/src/tests/messageapi/storage_message_address_test.cpp index f7f254e9119..edcf795368a 100644 --- a/storageapi/src/tests/messageapi/storage_message_address_test.cpp +++ b/storageapi/src/tests/messageapi/storage_message_address_test.cpp @@ -9,8 +9,8 @@ namespace storage::api { namespace { -size_t hash_of(vespalib::stringref cluster, const lib::NodeType& type, uint16_t index) { - return StorageMessageAddress(cluster, type, index).internal_storage_hash(); +size_t hash_of(const vespalib::string & cluster, const lib::NodeType& type, uint16_t index) { + return StorageMessageAddress(&cluster, type, index).internal_storage_hash(); } } @@ -32,8 +32,8 @@ TEST(StorageMessageAddressTest, storage_hash_covers_all_expected_fields) { EXPECT_NE(hash_of("foo", lib::NodeType::STORAGE, 0), hash_of("foo", lib::NodeType::STORAGE, 1)); - EXPECT_EQ(88u, sizeof(StorageMessageAddress)); - EXPECT_EQ(64u, sizeof(StorageMessage)); + EXPECT_EQ(16u, sizeof(StorageMessageAddress)); + EXPECT_EQ(72u, sizeof(StorageMessage)); EXPECT_EQ(16u, sizeof(mbus::Trace)); } diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp index 9c5df379d22..978b5d4a7e4 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp @@ -143,39 +143,47 @@ std::ostream & operator << (std::ostream & os, const StorageMessageAddress & add return os << addr.toString(); } -static vespalib::string -createAddress(vespalib::stringref cluster, const lib::NodeType& type, uint16_t index) -{ +namespace { + +vespalib::string +createAddress(vespalib::stringref cluster, const lib::NodeType &type, uint16_t index) { vespalib::asciistream os; os << STORAGEADDRESS_PREFIX << cluster << '/' << type.toString() << '/' << index << "/default"; return os.str(); } -size_t -calculate_node_hash(const lib::NodeType& type, uint16_t index) -{ - uint16_t buf[] = { type, index }; - return vespalib::hashValue(&buf, sizeof(buf)); +uint32_t +calculate_node_hash(const lib::NodeType &type, uint16_t index) { + uint16_t buf[] = {type, index}; + size_t hash = vespalib::hashValue(&buf, sizeof(buf)); + return uint32_t(hash & 0xffffffffl) ^ uint32_t(hash >> 32); +} + +vespalib::string Empty; + } // TODO we ideally want this removed. Currently just in place to support usage as map key when emplacement not available StorageMessageAddress::StorageMessageAddress() - : _cluster(), + : _cluster(&Empty), _precomputed_storage_hash(0), - _type(nullptr), + _type(lib::NodeType::Type::UNKNOWN), _protocol(Protocol::STORAGE), _index(0) {} -StorageMessageAddress::StorageMessageAddress(vespalib::stringref cluster, const lib::NodeType& type, +StorageMessageAddress::StorageMessageAddress(const vespalib::string * cluster, const lib::NodeType& type, uint16_t index) + : StorageMessageAddress(cluster, type, index, Protocol::STORAGE) +{ } + +StorageMessageAddress::StorageMessageAddress(const vespalib::string * cluster, const lib::NodeType& type, uint16_t index, Protocol protocol) : _cluster(cluster), _precomputed_storage_hash(calculate_node_hash(type, index)), - _type(&type), + _type(type.getType()), _protocol(protocol), _index(index) -{ -} +{ } StorageMessageAddress::~StorageMessageAddress() = default; @@ -183,50 +191,20 @@ mbus::Route StorageMessageAddress::to_mbus_route() const { mbus::Route result; - auto address_as_str = createAddress(_cluster, *_type, _index); + auto address_as_str = createAddress(getCluster(), lib::NodeType::get(_type), _index); std::vector<mbus::IHopDirective::SP> directives; directives.emplace_back(std::make_shared<mbus::VerbatimDirective>(std::move(address_as_str))); result.addHop(mbus::Hop(std::move(directives), false)); return result; } -uint16_t -StorageMessageAddress::getIndex() const -{ - if (!_type) { - throw vespalib::IllegalStateException("Cannot retrieve node index out of external address", VESPA_STRLOC); - } - return _index; -} - -const lib::NodeType& -StorageMessageAddress::getNodeType() const -{ - if (!_type) { - throw vespalib::IllegalStateException("Cannot retrieve node type out of external address", VESPA_STRLOC); - } - return *_type; -} - -const vespalib::string& -StorageMessageAddress::getCluster() const -{ - if (!_type) { - throw vespalib::IllegalStateException("Cannot retrieve cluster out of external address", VESPA_STRLOC); - } - return _cluster; -} - bool StorageMessageAddress::operator==(const StorageMessageAddress& other) const { if (_protocol != other._protocol) return false; if (_type != other._type) return false; - if (_type) { - if (_index != other._index) return false; - if (_type != other._type) return false; - if (_cluster != other._cluster) return false; - } + if (_index != other._index) return false; + if (getCluster() != other.getCluster()) return false; return true; } @@ -242,15 +220,15 @@ void StorageMessageAddress::print(vespalib::asciistream & out) const { out << "StorageMessageAddress("; - if (_protocol == STORAGE) { + if (_protocol == Protocol::STORAGE) { out << "Storage protocol"; } else { out << "Document protocol"; } - if (!_type) { + if (_type == lib::NodeType::Type::UNKNOWN) { out << ", " << to_mbus_route().toString() << ")"; } else { - out << ", cluster " << _cluster << ", nodetype " << *_type + out << ", cluster " << getCluster() << ", nodetype " << lib::NodeType::get(_type) << ", index " << _index << ")"; } } diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h index 98552e473c1..21d2d1b8e13 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h @@ -263,40 +263,44 @@ public: */ class StorageMessageAddress { public: - enum Protocol { STORAGE, DOCUMENT }; + enum class Protocol : uint8_t { STORAGE, DOCUMENT }; private: - vespalib::string _cluster; + const vespalib::string *_cluster; // Used for internal VDS addresses only - size_t _precomputed_storage_hash; - const lib::NodeType* _type; - Protocol _protocol; - uint16_t _index; + uint32_t _precomputed_storage_hash; + lib::NodeType::Type _type; + Protocol _protocol; + uint16_t _index; public: StorageMessageAddress(); // Only to be used when transient default ctor semantics are needed by containers - StorageMessageAddress(vespalib::stringref clusterName, - const lib::NodeType& type, uint16_t index, - Protocol protocol = STORAGE); + StorageMessageAddress(const vespalib::string * clusterName, const lib::NodeType& type, uint16_t index); + StorageMessageAddress(const vespalib::string * cluster, const lib::NodeType& type, uint16_t index, Protocol protocol); ~StorageMessageAddress(); void setProtocol(Protocol p) { _protocol = p; } mbus::Route to_mbus_route() const; Protocol getProtocol() const { return _protocol; } - uint16_t getIndex() const; - const lib::NodeType& getNodeType() const; - const vespalib::string& getCluster() const; + uint16_t getIndex() const { return _index; } + lib::NodeType::Type getNodeType() const { return _type; } + const vespalib::string& getCluster() const { return *_cluster; } // Returns precomputed hash over <type, index> pair. Other fields not included. - [[nodiscard]] size_t internal_storage_hash() const noexcept { + [[nodiscard]] uint32_t internal_storage_hash() const noexcept { return _precomputed_storage_hash; } bool operator==(const StorageMessageAddress& other) const; vespalib::string toString() const; friend std::ostream & operator << (std::ostream & os, const StorageMessageAddress & addr); - + static StorageMessageAddress create(const vespalib::string * cluster, const lib::NodeType& type, uint16_t index) { + return api::StorageMessageAddress(cluster, type, index); + } + static StorageMessageAddress createDocApi(const vespalib::string * cluster, const lib::NodeType& type, uint16_t index) { + return api::StorageMessageAddress(cluster, type, index, Protocol::DOCUMENT); + } private: void print(vespalib::asciistream & out) const; }; @@ -354,12 +358,12 @@ private: protected: static Id generateMsgId(); - const MessageType& _type; - Id _msgId; - std::unique_ptr<StorageMessageAddress> _address; - vespalib::Trace _trace; - uint32_t _approxByteSize; - Priority _priority; + const MessageType& _type; + Id _msgId; + StorageMessageAddress _address; + vespalib::Trace _trace; + uint32_t _approxByteSize; + Priority _priority; StorageMessage(const MessageType& code, Id id); StorageMessage(const StorageMessage&, Id id); @@ -386,10 +390,10 @@ public: void setPriority(Priority p) { _priority = p; } Priority getPriority() const { return _priority; } - const StorageMessageAddress* getAddress() const { return _address.get(); } + const StorageMessageAddress* getAddress() const { return (_address.getNodeType() != lib::NodeType::Type::UNKNOWN) ? &_address : nullptr; } void setAddress(const StorageMessageAddress& address) { - _address = std::make_unique<StorageMessageAddress>(address); + _address = address; } /** diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp index 2bb9fabd7d5..63749c5b341 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp @@ -12,7 +12,7 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code) { setPriority(cmd.getPriority()); if (cmd.getAddress()) { - setAddress(*cmd.getAddress()); + setAddress(*cmd.getAddress()); // Hmm, could we steal the address ? } // TODD do we really need copy construction if ( ! cmd.getTrace().isEmpty()) { @@ -26,10 +26,8 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code) StorageReply::~StorageReply() = default; void -StorageReply::print(std::ostream& out, bool verbose, - const std::string& indent) const +StorageReply::print(std::ostream& out, bool , const std::string& ) const { - (void) verbose; (void) indent; out << "StorageReply(" << _type.getName() << ", " << _result << ")"; } diff --git a/vdslib/src/vespa/vdslib/state/nodetype.cpp b/vdslib/src/vespa/vdslib/state/nodetype.cpp index 915b1548f88..97e15625186 100644 --- a/vdslib/src/vespa/vdslib/state/nodetype.cpp +++ b/vdslib/src/vespa/vdslib/state/nodetype.cpp @@ -3,15 +3,15 @@ #include "nodetype.h" #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <cassert> -namespace storage { -namespace lib { +namespace storage::lib { // WARNING: Because static initialization happen in random order, the State // class use these enum values directly during static initialization since // these objects may yet not exist. Update in State if you change this. -const NodeType NodeType::STORAGE("storage", 0); -const NodeType NodeType::DISTRIBUTOR("distributor", 1); +const NodeType NodeType::STORAGE("storage", NodeType::Type::STORAGE); +const NodeType NodeType::DISTRIBUTOR("distributor", NodeType::Type::DISTRIBUTOR); const NodeType& NodeType::get(vespalib::stringref serialized) @@ -26,8 +26,22 @@ NodeType::get(vespalib::stringref serialized) "Unknown node type " + serialized + " given.", VESPA_STRLOC); } -NodeType::NodeType(vespalib::stringref name, uint16_t enumValue) - : _enumValue(enumValue), _name(name) +const NodeType& +NodeType::get(Type type) +{ + switch (type) { + case Type::STORAGE: + return STORAGE; + case Type::DISTRIBUTOR: + return DISTRIBUTOR; + case Type::UNKNOWN: + assert(type != Type::UNKNOWN); + } + abort(); +} + +NodeType::NodeType(vespalib::stringref name, Type type) + : _type(type), _name(name) { } @@ -39,5 +53,4 @@ vespalib::asciistream & operator << (vespalib::asciistream & os, const NodeType return os << n.toString(); } -} // lib -} // storage +} diff --git a/vdslib/src/vespa/vdslib/state/nodetype.h b/vdslib/src/vespa/vdslib/state/nodetype.h index 35edd83fdac..abf1c5709b6 100644 --- a/vdslib/src/vespa/vdslib/state/nodetype.h +++ b/vdslib/src/vespa/vdslib/state/nodetype.h @@ -10,31 +10,30 @@ #pragma once #include <vespa/vespalib/stllike/string.h> -#include <stdint.h> namespace vespalib { class asciistream; } -namespace storage { -namespace lib { +namespace storage::lib { class NodeType { - typedef vespalib::asciistream asciistream; - uint16_t _enumValue; - vespalib::string _name; - - NodeType(vespalib::stringref name, uint16_t enumValue); - public: + NodeType(const NodeType &) = delete; + NodeType & operator = (const NodeType &) = delete; + NodeType(NodeType &&) = delete; + NodeType & operator =(NodeType &&) = delete; + enum class Type : uint8_t {STORAGE = 0, DISTRIBUTOR = 1, UNKNOWN = 2}; static const NodeType DISTRIBUTOR; static const NodeType STORAGE; /** Throws vespalib::IllegalArgumentException if invalid state given. */ static const NodeType& get(vespalib::stringref serialized); + static const NodeType& get(Type type); const vespalib::string& serialize() const { return _name; } - operator uint16_t() const { return _enumValue; } + Type getType() const { return _type; } + operator uint16_t() const { return static_cast<uint16_t>(_type); } const vespalib::string & toString() const { return _name; } bool operator==(const NodeType& other) const { return (&other == this); } @@ -43,11 +42,15 @@ public: bool operator<(const NodeType& other) const { return (&other == this ? false : *this == NodeType::DISTRIBUTOR); } +private: + Type _type; + vespalib::string _name; + + NodeType(vespalib::stringref name, Type type); }; std::ostream & operator << (std::ostream & os, const NodeType & n); vespalib::asciistream & operator << (vespalib::asciistream & os, const NodeType & n); -} // lib -} // storage +} |