SERVER-16444 Change updateWithDamages() to return the updated data.

Enable the UpdateStage to perform an in-place update regardless of
whether the data underlying the WorkingSetMember is an unowned BSONObj
or an owned BSONObj.

Change updateWithDamagesSupported() to return true for the
inMemoryExperiment storage engine.

14 files changed, 92 insertions, 83 deletions

diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
index 3b399b89bbb..54c511fd2a2 100644
--- a/src/mongo/db/catalog/collection.cpp
+++ b/src/mongo/db/catalog/collection.cpp
@@ -597,12 +597,13 @@ bool Collection::updateWithDamagesSupported() const {
     return _recordStore->updateWithDamagesSupported();
 }
 
-Status Collection::updateDocumentWithDamages(OperationContext* txn,
-                                             const RecordId& loc,
-                                             const Snapshotted<RecordData>& oldRec,
-                                             const char* damageSource,
-                                             const mutablebson::DamageVector& damages,
-                                             oplogUpdateEntryArgs& args) {
+StatusWith<RecordData> Collection::updateDocumentWithDamages(
+    OperationContext* txn,
+    const RecordId& loc,
+    const Snapshotted<RecordData>& oldRec,
+    const char* damageSource,
+    const mutablebson::DamageVector& damages,
+    oplogUpdateEntryArgs& args) {
     dassert(txn->lockState()->isCollectionLockedForMode(ns().toString(), MODE_IX));
     invariant(oldRec.snapshotId() == txn->recoveryUnit()->getSnapshotId());
     invariant(updateWithDamagesSupported());
@@ -610,14 +611,14 @@ Status Collection::updateDocumentWithDamages(OperationContext* txn,
     // Broadcast the mutation so that query results stay correct.
     _cursorManager.invalidateDocument(txn, loc, INVALIDATION_MUTATION);
 
-    Status status =
+    auto newRecStatus =
         _recordStore->updateWithDamages(txn, loc, oldRec.value(), damageSource, damages);
 
-    if (status.isOK()) {
+    if (newRecStatus.isOK()) {
         args.ns = ns().ns();
         getGlobalServiceContext()->getOpObserver()->onUpdate(txn, args);
     }
-    return status;
+    return newRecStatus;
 }
 
 bool Collection::_enforceQuota(bool userEnforeQuota) const {
diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index 19cc8287fa0..9c1921122bd 100644
--- a/src/mongo/db/catalog/collection.h
+++ b/src/mongo/db/catalog/collection.h
@@ -261,13 +261,14 @@ public:
     /**
      * Not allowed to modify indexes.
      * Illegal to call if updateWithDamagesSupported() returns false.
+     * @return the contents of the updated record.
      */
-    Status updateDocumentWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const Snapshotted<RecordData>& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages,
-                                     oplogUpdateEntryArgs& args);
+    StatusWith<RecordData> updateDocumentWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const Snapshotted<RecordData>& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages,
+                                                     oplogUpdateEntryArgs& args);
 
     // -----------
 
diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp
index 05c44190b5e..62580187790 100644
--- a/src/mongo/db/exec/update.cpp
+++ b/src/mongo/db/exec/update.cpp
@@ -530,13 +530,14 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco
                 args.update = logObj;
                 args.criteria = idQuery;
                 args.fromMigrate = request->isFromMigration();
-                _collection->updateDocumentWithDamages(
+                StatusWith<RecordData> newRecStatus = _collection->updateDocumentWithDamages(
                     getOpCtx(),
                     loc,
                     Snapshotted<RecordData>(oldObj.snapshotId(), oldRec),
                     source,
                     _damages,
                     args);
+                newObj = uassertStatusOK(std::move(newRecStatus)).releaseToBson();
             }
 
             _specificStats.fastmod = true;
diff --git a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
index 25a61a9edb2..bf8f8704007 100644
--- a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
+++ b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp
@@ -124,11 +124,11 @@ public:
         return false;
     }
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages) {
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages) {
         invariant(false);
     }
 
diff --git a/src/mongo/db/storage/in_memory/in_memory_record_store.cpp b/src/mongo/db/storage/in_memory/in_memory_record_store.cpp
index 7c3b708d513..65d7be83300 100644
--- a/src/mongo/db/storage/in_memory/in_memory_record_store.cpp
+++ b/src/mongo/db/storage/in_memory/in_memory_record_store.cpp
@@ -463,21 +463,15 @@ StatusWith<RecordId> InMemoryRecordStore::updateRecord(OperationContext* txn,
 }
 
 bool InMemoryRecordStore::updateWithDamagesSupported() const {
-    // TODO: Currently the UpdateStage assumes that updateWithDamages will apply the
-    // damages directly to the unowned BSONObj containing the record to be modified.
-    // The implementation of updateWithDamages() below copies the old record to a
-    // a new one and then applies the damages.
-    //
-    // We should be able to enable updateWithDamages() here once this assumption is
-    // relaxed.
-    return false;
+    return true;
 }
 
-Status InMemoryRecordStore::updateWithDamages(OperationContext* txn,
-                                              const RecordId& loc,
-                                              const RecordData& oldRec,
-                                              const char* damageSource,
-                                              const mutablebson::DamageVector& damages) {
+StatusWith<RecordData> InMemoryRecordStore::updateWithDamages(
+    OperationContext* txn,
+    const RecordId& loc,
+    const RecordData& oldRec,
+    const char* damageSource,
+    const mutablebson::DamageVector& damages) {
     InMemoryRecord* oldRecord = recordFor(loc);
     const int len = oldRecord->size;
 
@@ -500,7 +494,7 @@ Status InMemoryRecordStore::updateWithDamages(OperationContext* txn,
 
     *oldRecord = newRecord;
 
-    return Status::OK();
+    return newRecord.toRecordData();
 }
 
 std::unique_ptr<RecordCursor> InMemoryRecordStore::getCursor(OperationContext* txn,
diff --git a/src/mongo/db/storage/in_memory/in_memory_record_store.h b/src/mongo/db/storage/in_memory/in_memory_record_store.h
index bd241555394..1c13c729383 100644
--- a/src/mongo/db/storage/in_memory/in_memory_record_store.h
+++ b/src/mongo/db/storage/in_memory/in_memory_record_store.h
@@ -78,11 +78,11 @@ public:
 
     virtual bool updateWithDamagesSupported() const;
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages);
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages);
 
     std::unique_ptr<RecordCursor> getCursor(OperationContext* txn, bool forward) const final;
 
diff --git a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
index aa193549440..0fa4d919270 100644
--- a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
+++ b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h
@@ -87,11 +87,11 @@ public:
         return true;
     }
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages) {
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages) {
         invariant(false);
     }
 
diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
index 69abb4c872a..3240ecb491a 100644
--- a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
+++ b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
@@ -417,11 +417,12 @@ bool RecordStoreV1Base::updateWithDamagesSupported() const {
     return true;
 }
 
-Status RecordStoreV1Base::updateWithDamages(OperationContext* txn,
-                                            const RecordId& loc,
-                                            const RecordData& oldRec,
-                                            const char* damageSource,
-                                            const mutablebson::DamageVector& damages) {
+StatusWith<RecordData> RecordStoreV1Base::updateWithDamages(
+    OperationContext* txn,
+    const RecordId& loc,
+    const RecordData& oldRec,
+    const char* damageSource,
+    const mutablebson::DamageVector& damages) {
     MmapV1RecordHeader* rec = recordFor(DiskLoc::fromRecordId(loc));
     char* root = rec->data();
 
@@ -434,7 +435,7 @@ Status RecordStoreV1Base::updateWithDamages(OperationContext* txn,
         std::memcpy(targetPtr, sourcePtr, where->size);
     }
 
-    return Status::OK();
+    return rec->toRecordData();
 }
 
 void RecordStoreV1Base::deleteRecord(OperationContext* txn, const RecordId& rid) {
diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_base.h b/src/mongo/db/storage/mmap_v1/record_store_v1_base.h
index 7ba485d2a8b..3ba8d0c96c8 100644
--- a/src/mongo/db/storage/mmap_v1/record_store_v1_base.h
+++ b/src/mongo/db/storage/mmap_v1/record_store_v1_base.h
@@ -208,11 +208,11 @@ public:
 
     virtual bool updateWithDamagesSupported() const;
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages);
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages);
 
     virtual std::unique_ptr<RecordCursor> getCursorForRepair(OperationContext* txn) const;
 
diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h
index 2c73fc8252a..a4e550143f2 100644
--- a/src/mongo/db/storage/record_store.h
+++ b/src/mongo/db/storage/record_store.h
@@ -384,11 +384,11 @@ public:
      */
     virtual bool updateWithDamagesSupported() const = 0;
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages) = 0;
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages) = 0;
 
     /**
      * Returns a new cursor over this record store.
diff --git a/src/mongo/db/storage/record_store_test_harness.cpp b/src/mongo/db/storage/record_store_test_harness.cpp
index b448bc7d1c0..95fd4c993b0 100644
--- a/src/mongo/db/storage/record_store_test_harness.cpp
+++ b/src/mongo/db/storage/record_store_test_harness.cpp
@@ -311,8 +311,10 @@ TEST(RecordStoreTestHarness, UpdateInPlace1) {
             dv[0].sourceOffset = 0;
             dv[0].targetOffset = 3;
             dv[0].size = 3;
-            Status res = rs->updateWithDamages(opCtx.get(), loc, s1Rec, damageSource, dv);
-            ASSERT_OK(res);
+
+            auto newRecStatus = rs->updateWithDamages(opCtx.get(), loc, s1Rec, damageSource, dv);
+            ASSERT_OK(newRecStatus.getStatus());
+            ASSERT_EQUALS(s2, newRecStatus.getValue().data());
             uow.commit();
         }
     }
diff --git a/src/mongo/db/storage/record_store_test_updatewithdamages.cpp b/src/mongo/db/storage/record_store_test_updatewithdamages.cpp
index 701d5fd58f0..efbac5038d7 100644
--- a/src/mongo/db/storage/record_store_test_updatewithdamages.cpp
+++ b/src/mongo/db/storage/record_store_test_updatewithdamages.cpp
@@ -73,6 +73,7 @@ TEST(RecordStoreTestHarness, UpdateWithDamages) {
         ASSERT_EQUALS(1, rs->numRecords(opCtx.get()));
     }
 
+    string modifiedData = "11101000";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
@@ -88,17 +89,18 @@ TEST(RecordStoreTestHarness, UpdateWithDamages) {
             dv[2].size = 3;
 
             WriteUnitOfWork uow(opCtx.get());
-            ASSERT_OK(rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv));
+            auto newRecStatus = rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv);
+            ASSERT_OK(newRecStatus.getStatus());
+            ASSERT_EQUALS(modifiedData, newRecStatus.getValue().data());
             uow.commit();
         }
     }
 
-    data = "11101000";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
             RecordData record = rs->dataFor(opCtx.get(), loc);
-            ASSERT_EQUALS(data, record.data());
+            ASSERT_EQUALS(modifiedData, record.data());
         }
     }
 }
@@ -136,6 +138,7 @@ TEST(RecordStoreTestHarness, UpdateWithOverlappingDamageEvents) {
         ASSERT_EQUALS(1, rs->numRecords(opCtx.get()));
     }
 
+    string modifiedData = "10100010";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
@@ -148,17 +151,18 @@ TEST(RecordStoreTestHarness, UpdateWithOverlappingDamageEvents) {
             dv[1].size = 5;
 
             WriteUnitOfWork uow(opCtx.get());
-            ASSERT_OK(rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv));
+            auto newRecStatus = rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv);
+            ASSERT_OK(newRecStatus.getStatus());
+            ASSERT_EQUALS(modifiedData, newRecStatus.getValue().data());
             uow.commit();
         }
     }
 
-    data = "10100010";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
             RecordData record = rs->dataFor(opCtx.get(), loc);
-            ASSERT_EQUALS(data, record.data());
+            ASSERT_EQUALS(modifiedData, record.data());
         }
     }
 }
@@ -197,6 +201,7 @@ TEST(RecordStoreTestHarness, UpdateWithOverlappingDamageEventsReversed) {
         ASSERT_EQUALS(1, rs->numRecords(opCtx.get()));
     }
 
+    string modifiedData = "10111010";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
@@ -209,17 +214,18 @@ TEST(RecordStoreTestHarness, UpdateWithOverlappingDamageEventsReversed) {
             dv[1].size = 5;
 
             WriteUnitOfWork uow(opCtx.get());
-            ASSERT_OK(rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv));
+            auto newRecStatus = rs->updateWithDamages(opCtx.get(), loc, rec, data.c_str(), dv);
+            ASSERT_OK(newRecStatus.getStatus());
+            ASSERT_EQUALS(modifiedData, newRecStatus.getValue().data());
             uow.commit();
         }
     }
 
-    data = "10111010";
     {
         unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext());
         {
             RecordData record = rs->dataFor(opCtx.get(), loc);
-            ASSERT_EQUALS(data, record.data());
+            ASSERT_EQUALS(modifiedData, record.data());
         }
     }
 }
@@ -262,7 +268,9 @@ TEST(RecordStoreTestHarness, UpdateWithNoDamages) {
             mutablebson::DamageVector dv;
 
             WriteUnitOfWork uow(opCtx.get());
-            ASSERT_OK(rs->updateWithDamages(opCtx.get(), loc, rec, "", dv));
+            auto newRecStatus = rs->updateWithDamages(opCtx.get(), loc, rec, "", dv);
+            ASSERT_OK(newRecStatus.getStatus());
+            ASSERT_EQUALS(data, newRecStatus.getValue().data());
             uow.commit();
         }
     }
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
index c9955bcc609..a24676bcb52 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
@@ -928,11 +928,12 @@ bool WiredTigerRecordStore::updateWithDamagesSupported() const {
     return false;
 }
 
-Status WiredTigerRecordStore::updateWithDamages(OperationContext* txn,
-                                                const RecordId& loc,
-                                                const RecordData& oldRec,
-                                                const char* damageSource,
-                                                const mutablebson::DamageVector& damages) {
+StatusWith<RecordData> WiredTigerRecordStore::updateWithDamages(
+    OperationContext* txn,
+    const RecordId& loc,
+    const RecordData& oldRec,
+    const char* damageSource,
+    const mutablebson::DamageVector& damages) {
     invariant(false);
 }
 
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
index e39662cbcf6..557171ae59d 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h
@@ -131,11 +131,11 @@ public:
 
     virtual bool updateWithDamagesSupported() const;
 
-    virtual Status updateWithDamages(OperationContext* txn,
-                                     const RecordId& loc,
-                                     const RecordData& oldRec,
-                                     const char* damageSource,
-                                     const mutablebson::DamageVector& damages);
+    virtual StatusWith<RecordData> updateWithDamages(OperationContext* txn,
+                                                     const RecordId& loc,
+                                                     const RecordData& oldRec,
+                                                     const char* damageSource,
+                                                     const mutablebson::DamageVector& damages);
 
     std::unique_ptr<RecordCursor> getCursor(OperationContext* txn, bool forward) const final;
     std::unique_ptr<RecordCursor> getRandomCursor(OperationContext* txn) const final;