aboutsummaryrefslogtreecommitdiffstats
path: root/storage
diff options
context:
space:
mode:
authorTor Brede Vekterli <vekterli@yahooinc.com>2021-12-22 13:42:42 +0000
committerTor Brede Vekterli <vekterli@yahooinc.com>2021-12-22 16:50:02 +0000
commitbd3b312a489bbc2e41c94ca7c1cf65257830f27c (patch)
tree21da9cfa0b0dc0fe1cea0418681f22211aba2a67 /storage
parent84b08966eb755bdb3a3c96f622aa7e5267d4334c (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.cpp8
-rw-r--r--storage/src/vespa/storage/storageserver/communicationmanager.cpp5
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());