diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2021-12-22 13:42:42 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@yahooinc.com> | 2021-12-22 16:50:02 +0000 |
commit | bd3b312a489bbc2e41c94ca7c1cf65257830f27c (patch) | |
tree | 21da9cfa0b0dc0fe1cea0418681f22211aba2a67 /storage | |
parent | 84b08966eb755bdb3a3c96f622aa7e5267d4334c (diff) |
Let CommunicationManager swallow any errant internal reply messages
It's possible for internal commands to have replies auto-generated
if they're being evicted/aborted from persistence queue structures.
Most of these replies are intercepted by higher-level links in the
storage chain, but commands such as `RunTaskCommand` are actually
initiated by the persistence _provider_ and not a higher level component,
and are therefore not caught explicitly anywhere.
Let CommunicationManager signal that internal replies are handled,
even if the handling of these is just to ignore them entirely. This
prevents the replies from being spuriously warning-logged as "unhandled
message on top of call chain".
Diffstat (limited to 'storage')
-rw-r--r-- | storage/src/tests/storageserver/communicationmanagertest.cpp | 8 | ||||
-rw-r--r-- | storage/src/vespa/storage/storageserver/communicationmanager.cpp | 5 |
2 files changed, 12 insertions, 1 deletions
diff --git a/storage/src/tests/storageserver/communicationmanagertest.cpp b/storage/src/tests/storageserver/communicationmanagertest.cpp index 3e08a6d3901..ba5e0c3c116 100644 --- a/storage/src/tests/storageserver/communicationmanagertest.cpp +++ b/storage/src/tests/storageserver/communicationmanagertest.cpp @@ -6,6 +6,7 @@ #include <vespa/messagebus/rpcmessagebus.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h> +#include <vespa/storage/persistence/messages.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <tests/common/teststorageapp.h> #include <tests/common/dummystoragelink.h> @@ -331,4 +332,11 @@ TEST_F(CommunicationManagerTest, unmapped_bucket_space_for_get_documentapi_reque EXPECT_EQ(uint64_t(1), f.comm_mgr->metrics().bucketSpaceMappingFailures.getValue()); } +TEST_F(CommunicationManagerTest, communication_manager_swallows_internal_replies) { + CommunicationManagerFixture f; + auto msg = std::make_unique<RecheckBucketInfoCommand>(makeDocumentBucket({16, 1})); + auto reply = std::shared_ptr<api::StorageReply>(msg->makeReply()); + EXPECT_TRUE(f.comm_mgr->onUp(reply)); // true == handled by storage link +} + } // storage diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index c09908e93c7..9b67830b3dc 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -737,7 +737,10 @@ CommunicationManager::sendReply( if (!context) { LOG(spam, "No transport context in reply %s", reply->toString().c_str()); - return false; + // If it's an autogenerated reply for an internal message type, just throw it away + // by returning that we've handled it. No one else will handle the reply, the + // alternative is that it ends up as warning noise in the log. + return (reply->getType().getId() == api::MessageType::INTERNAL_REPLY_ID); } framework::MilliSecTimer startTime(_component.getClock()); |