MB-34367 [SR]: Correctly propogate IO-complete status from subdoc mutation

If the bucket_store() step of a sub-doc update operation returns
EWOULDBLOCK and then the subsequent notify_IO_complete has a
non-success status (for example a SyncWrite which timed out and notify
returns sync_write_ambiguous), the non-zero status is ignored.

Change-Id: I22dc3e5ea7dadc48e5cffead3aac01466b5f236f
Reviewed-on: http://review.couchbase.org/109911
Reviewed-by: Jim Walker <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: daverigby

daemon/subdocument.cc

@@ -1252,27 +1252,30 @@ static ENGINE_ERROR_CODE subdoc_update(SubdocCmdContext& context,
     uint64_t new_cas;
     mutation_descr_t mdt;
     auto new_op = context.needs_new_doc ? OPERATION_ADD : OPERATION_CAS;
-    if (context.do_delete_doc && context.no_sys_xattrs) {
-        new_cas = context.in_cas;
-        auto docKey = connection.makeDocKey(key);
-        ret = bucket_remove(cookie,
-                            docKey,
-                            new_cas,
-                            vbucket,
-                            cookie.getRequest(Cookie::PacketContent::Full)
-                                    .getDurabilityRequirements(),
-                            mdt);
-    } else {
-        ret = bucket_store(cookie,
-                           context.out_doc.get(),
-                           new_cas,
-                           new_op,
-                           cookie.getRequest(Cookie::PacketContent::Full)
-                                   .getDurabilityRequirements(),
-                           context.do_delete_doc ? DocumentState::Deleted
-                                                 : context.in_document_state);
-    }
-    ret = connection.remapErrorCode(ret);
+    if (ret == ENGINE_SUCCESS) {
+        if (context.do_delete_doc && context.no_sys_xattrs) {
+            new_cas = context.in_cas;
+            auto docKey = connection.makeDocKey(key);
+            ret = bucket_remove(cookie,
+                                docKey,
+                                new_cas,
+                                vbucket,
+                                cookie.getRequest(Cookie::PacketContent::Full)
+                                        .getDurabilityRequirements(),
+                                mdt);
+        } else {
+            ret = bucket_store(cookie,
+                               context.out_doc.get(),
+                               new_cas,
+                               new_op,
+                               cookie.getRequest(Cookie::PacketContent::Full)
+                                       .getDurabilityRequirements(),
+                               context.do_delete_doc
+                                       ? DocumentState::Deleted
+                                       : context.in_document_state);
+        }
+        ret = connection.remapErrorCode(ret);
+    }
     switch (ret) {
     case ENGINE_SUCCESS:
         // Record the UUID / Seqno if MUTATION_SEQNO feature is enabled so

tests/testapp/testapp.cc

@@ -968,6 +968,10 @@ void set_mutation_seqno_feature(bool enable) {
     set_feature(cb::mcbp::Feature::MUTATION_SEQNO, enable);
 }
 
+void set_xerror_feature(bool enable) {
+    set_feature(cb::mcbp::Feature::XERROR, enable);
+}
+
 void TestappTest::waitForShutdown(bool killed) {
 #ifdef WIN32
     ASSERT_EQ(WAIT_OBJECT_0,

tests/testapp/testapp.h

@@ -435,6 +435,9 @@ void set_json_feature(bool enable);
 // Enables / disables the MUTATION_SEQNO feature.
 void set_mutation_seqno_feature(bool enable);
 
+// Enables / disables the XERROR feature.
+void set_xerror_feature(bool enable);
+
 /* Send the specified buffer+len to memcached. */
 void safe_send(const void* buf, size_t len, bool hickup);
 

tests/testapp/testapp_subdoc.cc

@@ -2097,6 +2097,28 @@ TEST_P(SubdocTestappTest, MB_30278_SubdocBacktickDictAdd) {
     validate_json_document("doc", R"({"key`":"value","key2`":"value2"})");
 }
 
+// MB-34367: If a multi-mutation with durability requirements times out,
+// ensure the status is reported correctly.
+TEST_P(SubdocTestappTest, SubdocDurabilityTimeout) {
+    /// Need XERORR given we are using sync_write_ambiguous
+    set_xerror_feature(true);
+
+    // Setup ewouldblock_engine to simulate a timeout - first 3 calls should
+    // succeed, bucket_store should return ewouldblock and 'pending' IO return
+    // sync_write_ambiguous.
+    ewouldblock_engine_configure({cb::engine_errc(-1),
+                                  cb::engine_errc(-1),
+                                  cb::engine_errc::would_block,
+                                  cb::engine_errc::sync_write_ambiguous});
+    EXPECT_SD_ERR(BinprotSubdocCommand(cb::mcbp::ClientOpcode::SubdocDictUpsert,
+                                       "durability_timeout",
+                                       "element",
+                                       "4",
+                                       SUBDOC_FLAG_NONE,
+                                       mcbp::subdoc::doc_flag::Mkdoc),
+                  cb::mcbp::Status::SyncWriteAmbiguous);
+}
+
 INSTANTIATE_TEST_CASE_P(
         Subdoc,
         SubdocTestappTest,