Ensure missing documents on replicas are not erroneously considered consistent

Introducing a new member in the category "stupid bugs that I should have
added explicit tests for when adding the feature initially".

There was ambiguity in the GetOperation code where a timestamp sentinel
value of zero was used to denote not having received any replies yet,
but where a timestamp of zero also means "document not found on replica".
This means that if the first reply was from a replica _without_ a document
and the second reply was from a replica _with_ a document, the code
would act as if the first reply effectively did not exist. Consequently
the Get operation would be tagged as consistent. This had very bad
consequences for the two-phase update operation logic that relied on
this information to be correct.

This change ensures there is no ambiguity between not having received
a reply and having a received a reply with a missing document.

1 files changed, 15 insertions, 0 deletions

diff --git a/storage/src/tests/distributor/getoperationtest.cpp b/storage/src/tests/distributor/getoperationtest.cpp
index 72a124d45b4..db790639869 100644
--- a/storage/src/tests/distributor/getoperationtest.cpp
+++ b/storage/src/tests/distributor/getoperationtest.cpp
@@ -363,6 +363,21 @@ TEST_F(GetOperationTest, not_found) {
     EXPECT_TRUE(last_reply_had_consistent_replicas());
 }
 
+TEST_F(GetOperationTest, not_found_on_subset_of_replicas_marks_get_as_inconsistent) {
+    setClusterState("distributor:1 storage:2");
+    addNodesToBucketDB(bucketId, "0=100,1=200");
+    sendGet();
+    ASSERT_EQ("Get => 0,Get => 1", _sender.getCommands(true));
+
+    ASSERT_NO_FATAL_FAILURE(sendReply(0, api::ReturnCode::OK, "newauthor", 101));
+    ASSERT_NO_FATAL_FAILURE(sendReply(1, api::ReturnCode::OK, "", 0)); // Not found.
+
+    ASSERT_EQ("GetReply(BucketId(0x0000000000000000), id:ns:text/html::uri, "
+              "timestamp 101) ReturnCode(NONE)",
+              _sender.getLastReply());
+    EXPECT_FALSE(last_reply_had_consistent_replicas());
+}
+
 TEST_F(GetOperationTest, resend_on_storage_failure) {
     setClusterState("distributor:1 storage:3");