From 0879cd61b66c67d6f1aaedf27763ca06b3eb7cc0 Mon Sep 17 00:00:00 2001
From: Tor Brede Vekterli <vekterli@verizonmedia.com>
Date: Wed, 18 Sep 2019 14:22:24 +0000
Subject: Allow Get operations through when content node is in Maintenance mode

If Gets are bounced by Maintenance nodes, operations that take place
in a two-phase state transition window Up->Maintenance will be aborted.
---
 storage/src/tests/storageserver/bouncertest.cpp    | 23 ++++++++++++++++++----
 .../src/vespa/storage/storageserver/bouncer.cpp    |  7 +++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

(limited to 'storage/src')

diff --git a/storage/src/tests/storageserver/bouncertest.cpp b/storage/src/tests/storageserver/bouncertest.cpp
index c19d8814af4..29a56b234d0 100644
--- a/storage/src/tests/storageserver/bouncertest.cpp
+++ b/storage/src/tests/storageserver/bouncertest.cpp
@@ -118,6 +118,13 @@ BouncerTest::createDummyFeedMessage(api::Timestamp timestamp,
     return cmd;
 }
 
+std::shared_ptr<api::StorageCommand> create_dummy_get_message() {
+    return std::make_shared<api::GetCommand>(
+            document::Bucket(document::FixedBucketSpaces::default_space(), document::BucketId(0)),
+            document::DocumentId("id:ns:foo::bar"),
+            "[all]");
+}
+
 TEST_F(BouncerTest, future_timestamp) {
     EXPECT_EQ(0, _manager->metrics().clock_skew_aborts.getValue());
 
@@ -221,25 +228,25 @@ TEST_F(BouncerTest, do_not_reject_higher_prioritized_feed_messages_than_configur
     expectMessageNotBounced();
 }
 
-TEST_F(BouncerTest, rejection_threshold_is_exclusive) {
+TEST_F(BouncerTest, priority_rejection_threshold_is_exclusive) {
     configureRejectionThreshold(Priority(120));
     _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(120)));
     expectMessageNotBounced();
 }
 
-TEST_F(BouncerTest, only_reject_feed_messages_when_configured) {
+TEST_F(BouncerTest, only_priority_reject_feed_messages_when_configured) {
     configureRejectionThreshold(RejectionDisabledConfigValue);
     // A message with even the lowest priority should not be rejected.
     _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(255)));
     expectMessageNotBounced();
 }
 
-TEST_F(BouncerTest, rejection_is_disabled_by_default_in_config) {
+TEST_F(BouncerTest, priority_rejection_is_disabled_by_default_in_config) {
     _upper->sendDown(createDummyFeedMessage(11 * 1000000, Priority(255)));
     expectMessageNotBounced();
 }
 
-TEST_F(BouncerTest, read_only_operations_are_not_rejected) {
+TEST_F(BouncerTest, read_only_operations_are_not_priority_rejected) {
     configureRejectionThreshold(Priority(1));
     // StatBucket is an external operation, but it's not a mutating operation
     // and should therefore not be blocked.
@@ -320,5 +327,13 @@ TEST_F(BouncerTest, cluster_state_activation_commands_are_not_bounced) {
     expectMessageNotBounced();
 }
 
+TEST_F(BouncerTest, allow_get_operations_when_node_is_in_maintenance_mode) {
+    auto state = makeClusterStateBundle("version:10 distributor:3 storage:3 .2.s:m", {}); // Our index is 2
+    _node->getNodeStateUpdater().setClusterStateBundle(state);
+    _upper->sendDown(create_dummy_get_message());
+    expectMessageNotBounced();
+    EXPECT_EQ(0, _manager->metrics().unavailable_node_aborts.getValue());
+}
+
 } // storage
 
diff --git a/storage/src/vespa/storage/storageserver/bouncer.cpp b/storage/src/vespa/storage/storageserver/bouncer.cpp
index fdbfd553315..bc9708f70a2 100644
--- a/storage/src/vespa/storage/storageserver/bouncer.cpp
+++ b/storage/src/vespa/storage/storageserver/bouncer.cpp
@@ -263,6 +263,13 @@ Bouncer::onDown(const std::shared_ptr<api::StorageMessage>& msg)
     {
         return false;
     }
+    // Special case for point lookup Gets while node is in maintenance mode
+    // to allow reads to complete during two-phase cluster state transitions
+    if ((*state == lib::State::MAINTENANCE) && (type.getId() == api::MessageType::GET_ID) && clusterIsUp()) {
+        MBUS_TRACE(msg->getTrace(), 7, "Bouncer: node is in Maintenance mode, but letting Get through");
+        return false;
+    }
+
     const bool externalLoad = isExternalLoad(type);
     if (!isInAvailableState && !(isDistributor() && externalLoad)) {
         abortCommandForUnavailableNode(*msg, *state);
-- 
cgit v1.2.3