Support test-and-set for auto-create document updates

Has the obvious consistency caveats that if all your existing replicas are
down, the update will go through since the document from an weak consistency
perspective does not exist anywhere. But can be a useful feature if this
is an acceptable tradeoff.

7 files changed, 72 insertions, 29 deletions

diff --git a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
index 3c82931467e..a8771ddc28a 100644
--- a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
+++ b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp
@@ -61,7 +61,8 @@ class TwoPhaseUpdateOperationTest : public CppUnit::TestFixture,
     CPPUNIT_TEST(testSafePathConditionMatchSendsPutsWithUpdatedDoc);
     CPPUNIT_TEST(testSafePathConditionParseFailureFailsWithIllegalParamsError);
     CPPUNIT_TEST(testSafePathConditonUnknownDocTypeFailsWithIllegalParamsError);
-    CPPUNIT_TEST(testSafePathConditionWithMissingDocFailsWithTasError);
+    CPPUNIT_TEST(safe_path_condition_with_missing_doc_and_no_auto_create_fails_with_tas_error);
+    CPPUNIT_TEST(safe_path_condition_with_missing_doc_and_auto_create_sends_puts);
     CPPUNIT_TEST(testFastPathCloseEdgeSendsCorrectReply);
     CPPUNIT_TEST(testSafePathCloseEdgeSendsCorrectReply);
     CPPUNIT_TEST_SUITE_END();
@@ -97,7 +98,8 @@ protected:
     void testSafePathConditionMatchSendsPutsWithUpdatedDoc();
     void testSafePathConditionParseFailureFailsWithIllegalParamsError();
     void testSafePathConditonUnknownDocTypeFailsWithIllegalParamsError();
-    void testSafePathConditionWithMissingDocFailsWithTasError();
+    void safe_path_condition_with_missing_doc_and_no_auto_create_fails_with_tas_error();
+    void safe_path_condition_with_missing_doc_and_auto_create_sends_puts();
     void testFastPathCloseEdgeSendsCorrectReply();
     void testSafePathCloseEdgeSendsCorrectReply();
 
@@ -1096,7 +1098,7 @@ TwoPhaseUpdateOperationTest::testSafePathConditonUnknownDocTypeFailsWithIllegalP
 }
 
 void
-TwoPhaseUpdateOperationTest::testSafePathConditionWithMissingDocFailsWithTasError()
+TwoPhaseUpdateOperationTest::safe_path_condition_with_missing_doc_and_no_auto_create_fails_with_tas_error()
 {
     setupDistributor(2, 2, "storage:2 distributor:1");
     std::shared_ptr<TwoPhaseUpdateOperation> cb(
@@ -1118,6 +1120,22 @@ TwoPhaseUpdateOperationTest::testSafePathConditionWithMissingDocFailsWithTasErro
 }
 
 void
+TwoPhaseUpdateOperationTest::safe_path_condition_with_missing_doc_and_auto_create_sends_puts()
+{
+    setupDistributor(2, 2, "storage:2 distributor:1");
+    std::shared_ptr<TwoPhaseUpdateOperation> cb(
+            sendUpdate("0=1/2/3,1=2/3/4", UpdateOptions()
+                    .condition("testdoctype1.headerval==120")
+                    .createIfNonExistent(true)));
+
+    MessageSenderStub sender;
+    cb->start(sender, framework::MilliSecTime(0));
+    replyToGet(*cb, sender, 0, 100, false);
+    replyToGet(*cb, sender, 1, 110, false);
+    CPPUNIT_ASSERT_EQUAL("Put => 1,Put => 0"s, sender.getCommands(true, false, 2));
+}
+
+void
 TwoPhaseUpdateOperationTest::assertAbortedUpdateReplyWithContextPresent(
         const MessageSenderStub& closeSender) const
 {
@@ -1177,9 +1195,6 @@ TwoPhaseUpdateOperationTest::testSafePathCloseEdgeSendsCorrectReply()
 // document IDs without explicit doctypes will _not_ be auto-failed on the
 // distributor.
 
-// XXX shouldn't be necessary to have any special handling of create-if... and
-// test-and-set right? They appear fully mutually exclusive.
-
 // XXX: test case where update reply has been sent but callback still
 // has pending messages (e.g. n-of-m case).
 
diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp
index 686e10ba5ef..197aa95fc22 100644
--- a/storage/src/tests/persistence/testandsettest.cpp
+++ b/storage/src/tests/persistence/testandsettest.cpp
@@ -66,8 +66,10 @@ public:
     void conditional_remove_executed_on_condition_match();
     void conditional_update_not_executed_on_condition_mismatch();
     void conditional_update_executed_on_condition_match();
+    void conditional_update_not_executed_when_no_document_and_no_auto_create();
+    void conditional_update_executed_when_no_document_but_auto_create_is_enabled();
     void invalid_document_selection_should_fail();
-    void non_existing_document_should_fail();
+    void conditional_put_to_non_existing_document_should_fail();
     void document_with_no_type_should_fail();
 
     CPPUNIT_TEST_SUITE(TestAndSetTest);
@@ -77,16 +79,17 @@ public:
     CPPUNIT_TEST(conditional_remove_executed_on_condition_match);
     CPPUNIT_TEST(conditional_update_not_executed_on_condition_mismatch);
     CPPUNIT_TEST(conditional_update_executed_on_condition_match);
+    CPPUNIT_TEST(conditional_update_not_executed_when_no_document_and_no_auto_create);
+    CPPUNIT_TEST(conditional_update_executed_when_no_document_but_auto_create_is_enabled);
     CPPUNIT_TEST(invalid_document_selection_should_fail);
-    CPPUNIT_TEST(non_existing_document_should_fail);
+    CPPUNIT_TEST(conditional_put_to_non_existing_document_should_fail);
     CPPUNIT_TEST(document_with_no_type_should_fail);
     CPPUNIT_TEST_SUITE_END();
 
 protected:
     std::unique_ptr<api::UpdateCommand> conditional_update_test(
-        bool matchingHeader,
-        api::Timestamp timestampOne,
-        api::Timestamp timestampTwo);
+        bool createIfMissing,
+        api::Timestamp updateTimestamp);
 
     document::Document::SP createTestDocument();
     document::Document::SP retrieveTestDocument();
@@ -183,18 +186,16 @@ void TestAndSetTest::conditional_remove_executed_on_condition_match()
 }
 
 std::unique_ptr<api::UpdateCommand> TestAndSetTest::conditional_update_test(
-    bool matchingHeader,
-    api::Timestamp timestampOne,
-    api::Timestamp timestampTwo)
+    bool createIfMissing,
+    api::Timestamp updateTimestamp)
 {
-    putTestDocument(matchingHeader, timestampOne);
-
     auto docUpdate = std::make_shared<document::DocumentUpdate>(_env->_testDocMan.getTypeRepo(), testDoc->getType(), testDocId);
     auto fieldUpdate = document::FieldUpdate(testDoc->getField("content"));
     fieldUpdate.addUpdate(document::AssignValueUpdate(NEW_CONTENT));
     docUpdate->addUpdate(fieldUpdate);
+    docUpdate->setCreateIfNonExistent(createIfMissing);
 
-    auto updateUp = std::make_unique<api::UpdateCommand>(makeDocumentBucket(BUCKET_ID), docUpdate, timestampTwo);
+    auto updateUp = std::make_unique<api::UpdateCommand>(makeDocumentBucket(BUCKET_ID), docUpdate, updateTimestamp);
     setTestCondition(*updateUp);
     return updateUp;
 }
@@ -203,11 +204,12 @@ void TestAndSetTest::conditional_update_not_executed_on_condition_mismatch()
 {
     api::Timestamp timestampOne = 0;
     api::Timestamp timestampTwo = 1;
-    auto updateUp = conditional_update_test(false, timestampOne, timestampTwo);
+    putTestDocument(false, timestampOne);
+    auto updateUp = conditional_update_test(false, timestampTwo);
 
     CPPUNIT_ASSERT(thread->handleUpdate(*updateUp)->getResult() == api::ReturnCode::Result::TEST_AND_SET_CONDITION_FAILED);
     CPPUNIT_ASSERT_EQUAL(expectedDocEntryString(timestampOne, testDocId),
-                        dumpBucket(BUCKET_ID));
+                         dumpBucket(BUCKET_ID));
 
     assertTestDocumentFoundAndMatchesContent(OLD_CONTENT);
 }
@@ -216,7 +218,8 @@ void TestAndSetTest::conditional_update_executed_on_condition_match()
 {
     api::Timestamp timestampOne = 0;
     api::Timestamp timestampTwo = 1;
-    auto updateUp = conditional_update_test(true, timestampOne, timestampTwo);
+    putTestDocument(true, timestampOne);
+    auto updateUp = conditional_update_test(false, timestampTwo);
 
     CPPUNIT_ASSERT(thread->handleUpdate(*updateUp)->getResult() == api::ReturnCode::Result::OK);
     CPPUNIT_ASSERT_EQUAL(expectedDocEntryString(timestampOne, testDocId) +
@@ -226,6 +229,23 @@ void TestAndSetTest::conditional_update_executed_on_condition_match()
     assertTestDocumentFoundAndMatchesContent(NEW_CONTENT);
 }
 
+void TestAndSetTest::conditional_update_not_executed_when_no_document_and_no_auto_create() {
+    api::Timestamp updateTimestamp = 200;
+    auto updateUp = conditional_update_test(false, updateTimestamp);
+
+    CPPUNIT_ASSERT(thread->handleUpdate(*updateUp)->getResult() == api::ReturnCode::Result::TEST_AND_SET_CONDITION_FAILED);
+    CPPUNIT_ASSERT_EQUAL(""s, dumpBucket(BUCKET_ID));
+}
+
+void TestAndSetTest::conditional_update_executed_when_no_document_but_auto_create_is_enabled() {
+    api::Timestamp updateTimestamp = 200;
+    auto updateUp = conditional_update_test(true, updateTimestamp);
+
+    CPPUNIT_ASSERT(thread->handleUpdate(*updateUp)->getResult() == api::ReturnCode::Result::OK);
+    CPPUNIT_ASSERT_EQUAL(expectedDocEntryString(updateTimestamp, testDocId), dumpBucket(BUCKET_ID));
+    assertTestDocumentFoundAndMatchesContent(NEW_CONTENT);
+}
+
 void TestAndSetTest::invalid_document_selection_should_fail()
 {
     // Conditionally replace nonexisting document
@@ -238,7 +258,7 @@ void TestAndSetTest::invalid_document_selection_should_fail()
     CPPUNIT_ASSERT_EQUAL(""s, dumpBucket(BUCKET_ID));
 }
 
-void TestAndSetTest::non_existing_document_should_fail()
+void TestAndSetTest::conditional_put_to_non_existing_document_should_fail()
 {
     // Conditionally replace nonexisting document
     // Fail since no document exists to match with test and set
diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
index c652f787b2e..b7ebafc114c 100644
--- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
@@ -378,7 +378,7 @@ TwoPhaseUpdateOperation::handleSafePathReceivedGet(DistributorMessageSender& sen
         }
         docToUpdate = reply.getDocument();
         setUpdatedForTimestamp(receivedTimestamp);
-    } else if (hasTasCondition()) {
+    } else if (hasTasCondition() && !shouldCreateIfNonExistent()) {
         replyWithTasFailure(sender, "Document did not exist");
         return;
     } else if (shouldCreateIfNonExistent()) {
diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp
index 1b221ea0b7c..55f7c076244 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.cpp
+++ b/storage/src/vespa/storage/persistence/persistencethread.cpp
@@ -85,9 +85,10 @@ bool PersistenceThread::tasConditionExists(const api::TestAndSetCommand & cmd) {
     return cmd.getCondition().isPresent();
 }
 
-bool PersistenceThread::tasConditionMatches(const api::TestAndSetCommand & cmd, MessageTracker & tracker) {
+bool PersistenceThread::tasConditionMatches(const api::TestAndSetCommand & cmd, MessageTracker & tracker,
+                                            bool missingDocumentImpliesMatch) {
     try {
-        TestAndSetHelper helper(*this, cmd);
+        TestAndSetHelper helper(*this, cmd, missingDocumentImpliesMatch);
 
         auto code = helper.retrieveAndMatch();
         if (code.failed()) {
@@ -149,7 +150,7 @@ PersistenceThread::handleUpdate(api::UpdateCommand& cmd)
     auto tracker = std::make_unique<MessageTracker>(metrics, _env._component.getClock());
     metrics.request_size.addValue(cmd.getApproxByteSize());
 
-    if (tasConditionExists(cmd) && !tasConditionMatches(cmd, *tracker)) {
+    if (tasConditionExists(cmd) && !tasConditionMatches(cmd, *tracker, cmd.getUpdate()->getCreateIfNonExistent())) {
         return tracker;
     }
     
diff --git a/storage/src/vespa/storage/persistence/persistencethread.h b/storage/src/vespa/storage/persistence/persistencethread.h
index 05ca34f623d..ed27a759e8b 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.h
+++ b/storage/src/vespa/storage/persistence/persistencethread.h
@@ -87,7 +87,8 @@ private:
 
     friend class TestAndSetHelper;
     bool tasConditionExists(const api::TestAndSetCommand & cmd);
-    bool tasConditionMatches(const api::TestAndSetCommand & cmd, MessageTracker & tracker);
+    bool tasConditionMatches(const api::TestAndSetCommand & cmd, MessageTracker & tracker,
+                             bool missingDocumentImpliesMatch = false);
 };
 
 } // storage
diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp
index 70a6bdf21db..511d44ad331 100644
--- a/storage/src/vespa/storage/persistence/testandsethelper.cpp
+++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp
@@ -39,11 +39,13 @@ spi::GetResult TestAndSetHelper::retrieveDocument(const document::FieldSet & fie
         _thread._context);
 }
 
-TestAndSetHelper::TestAndSetHelper(PersistenceThread & thread, const api::TestAndSetCommand & cmd)
+TestAndSetHelper::TestAndSetHelper(PersistenceThread & thread, const api::TestAndSetCommand & cmd,
+                                   bool missingDocumentImpliesMatch)
     : _thread(thread),
       _component(thread._env._component),
       _cmd(cmd),
-      _docId(cmd.getDocumentId())
+      _docId(cmd.getDocumentId()),
+      _missingDocumentImpliesMatch(missingDocumentImpliesMatch)
 {
     getDocumentType();
     parseDocumentSelection();
@@ -69,6 +71,8 @@ api::ReturnCode TestAndSetHelper::retrieveAndMatch() {
 
         // Document matches
         return api::ReturnCode();
+    } else if (_missingDocumentImpliesMatch) {
+        return api::ReturnCode();
     }
 
     return api::ReturnCode(api::ReturnCode::TEST_AND_SET_CONDITION_FAILED, "Document does not exist");
diff --git a/storage/src/vespa/storage/persistence/testandsethelper.h b/storage/src/vespa/storage/persistence/testandsethelper.h
index 4e48259bc79..21c111c712f 100644
--- a/storage/src/vespa/storage/persistence/testandsethelper.h
+++ b/storage/src/vespa/storage/persistence/testandsethelper.h
@@ -26,13 +26,15 @@ class TestAndSetHelper {
     const document::DocumentId _docId;
     const document::DocumentType * _docTypePtr;
     std::unique_ptr<document::select::Node> _docSelectionUp;
+    bool _missingDocumentImpliesMatch;
 
     void getDocumentType();
     void parseDocumentSelection();
     spi::GetResult retrieveDocument(const document::FieldSet & fieldSet);
 
 public:
-    TestAndSetHelper(PersistenceThread & thread, const api::TestAndSetCommand & cmd);
+    TestAndSetHelper(PersistenceThread & thread, const api::TestAndSetCommand & cmd,
+                     bool missingDocumentImpliesMatch = false);
     ~TestAndSetHelper();
     api::ReturnCode retrieveAndMatch();
 };