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, 16 insertions, 0 deletions

diff --git a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
index 8f728c39bb8..c8bfcd754f7 100644
--- a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
+++ b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
@@ -1072,6 +1072,22 @@ TEST_F(TwoPhaseUpdateOperationTest, fast_path_not_restarted_if_replica_set_alter
     ASSERT_EQ("Put => 1,Put => 2,Put => 0", sender.getCommands(true, false, 2));
 }
 
+TEST_F(TwoPhaseUpdateOperationTest, fast_path_not_restarted_if_document_not_found_on_a_replica_node) {
+    setupDistributor(2, 2, "storage:2 distributor:1");
+    getConfig().set_update_fast_path_restart_enabled(true);
+
+    std::shared_ptr<TwoPhaseUpdateOperation> cb(sendUpdate("0=1/2/3,1=2/3/4")); // Inconsistent replicas.
+    DistributorMessageSenderStub sender;
+    cb->start(sender, framework::MilliSecTime(0));
+
+    ASSERT_EQ("Get => 0,Get => 1", sender.getCommands(true));
+    replyToGet(*cb, sender, 0, Timestamp(0), false);
+    replyToGet(*cb, sender, 1, Timestamp(500));
+
+    // Should _not_ send Update operations!
+    ASSERT_EQ("Put => 1,Put => 0", sender.getCommands(true, false, 2));
+}
+
 // XXX currently differs in behavior from content nodes in that updates for
 // document IDs without explicit doctypes will _not_ be auto-failed on the
 // distributor.