mongodb/mongo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SERVER-124369 Do not attempt to read diff from replacement-style update document (#53166)

Browse Source 
GitOrigin-RevId: e3a5db5e04abc2437088d35268b86a81bc00a4cc
This commit is contained in:
Justin Seyster 2026-05-08 18:34:28 -04:00 committed by MongoDB Bot
parent bb7b4816b5
commit 70746ba48a
18 changed files with 124 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
jstests/replsets/OWNERS.yml
View File

@ -51,3 +51,6 @@ filters:
- "*change_stream*":
approvers:
- 10gen/query-execution-change-streams
- "v2_delta_oplog_entries.js":
approvers:
- 10gen/query-execution

32
jstests/replsets/v2_delta_oplog_entries.js
View File

@ -6,6 +6,11 @@
* This test relies on the DBHash checker to run at the end to ensure that the primaries and
* secondaries have the same data. For that reason it's important that this test not drop
* intermediate collections.
*
* @tags: [
* # Only run this test on nodes that include the fix for SERVER-124369.
* requires_fcv_90,
* ]
*/
import {ReplSetTest} from "jstests/libs/replsettest.js";
@ -71,7 +76,10 @@ function checkOplogEntry(node, expectedOplogEntryType, expectedId) {
}
} else if (expectedOplogEntryType === kExpectReplacementEntry) {
assert.eq(oplogEntry.op, "u");
assert.eq(oplogEntry.o.hasOwnProperty("$v"), false, oplogEntry);
// A replacement-style update should be identifiable either by the presence of an '_id'
// field or the absence of a '$v' field.
assert(oplogEntry.o.hasOwnProperty("_id") || !oplogEntry.o.hasOwnProperty("$v"), oplogEntry);
} else if (expectedOplogEntryType == kExpectNoUpdateEntry) {
assert.eq(oplogEntry.op, "i");
assert.eq(oplogEntry.o._id, expectedId);
@ -334,7 +342,25 @@ testUpdateReplicates({
expectedOplogEntry: kExpectDeltaEntry,
});
// Don't drop any collections. At the end we want the DBHash checker will make sure there's no
// corruption.
// Verify that a replacement document containing '$v' and 'diff' fields does not get interpreted as
// a delta oplog entry (see SERVER-124369).
id = generateId();
testUpdateReplicates({
preImage: {_id: id, x: "foo", subObj: {a: 1, b: 2}},
pipeline: [{$replaceRoot: {newRoot: {$literal: {_id: id, x: kMediumLengthStr, "$v": 2, diff: {foo: "bar"}}}}}],
postImage: {_id: id, x: kMediumLengthStr, "$v": 2, diff: {foo: "bar"}},
expectedOplogEntry: kExpectReplacementEntry,
});
// Verify that a replacement document containing '$v' and 'diff' fields does not get interpreted as
// a delta oplog entry, even when the user-specified replacement does not explicitly set _id (see
// SERVER-124369).
id = generateId();
testUpdateReplicates({
preImage: {_id: id, x: "foo", subObj: {a: 1, b: 2}},
pipeline: [{$replaceRoot: {newRoot: {$literal: {x: kMediumLengthStr, "$v": 2, diff: {foo: "bar"}}}}}],
postImage: {_id: id, x: kMediumLengthStr, "$v": 2, diff: {foo: "bar"}},
expectedOplogEntry: kExpectReplacementEntry,
});
rst.stopSet();

10
src/mongo/db/auth/authorization_backend_mock.cpp
View File

@ -130,14 +130,8 @@ Status AuthorizationBackendMock::updateOne(OperationContext* opCtx,
const bool validateForStorage = false;
const FieldRefSet emptyImmutablePaths;
const bool isInsert = false;
BSONObj logObj;
status = driver.update(opCtx,
StringData(),
&document,
validateForStorage,
emptyImmutablePaths,
isInsert,
&logObj);
status = driver.update(
opCtx, StringData(), &document, validateForStorage, emptyImmutablePaths, isInsert);
if (!status.isOK())
return status;
BSONObj newObj = document.getObject().copy();

4
src/mongo/db/exec/classic/timeseries_upsert.cpp
View File

@ -112,9 +112,7 @@ PlanStage::StageState TimeseriesUpsertStage::doWork(WorkingSetID* out) {
return updateState;
}
// Since this is an insert, we will be logging it as such in the oplog. We don't need the
// driver's help to build the oplog record. We also set the 'nUpserted' stats counter here.
_params.updateDriver->setLogOp(false);
// Track this operation as an upserted measurement.
_specificStats.nMeasurementsUpserted = 1;
// Generate the new document to be inserted.

4
src/mongo/db/exec/classic/upsert_stage.cpp
View File

@ -124,10 +124,6 @@ PlanStage::StageState UpsertStage::doWork(WorkingSetID* out) {
// If the update resulted in EOF without matching anything, we must insert a new document.
invariant(updateState == PlanStage::IS_EOF && !isEOF());
// Since this is an insert, we will be logging it as such in the oplog. We don't need the
// driver's help to build the oplog record.
_params.driver->setLogOp(false);
// Generate the new document to be inserted.
auto newObj = _produceNewDocumentForInsert();

1
src/mongo/db/query/write_ops/parsed_update.cpp
View File

@ -124,7 +124,6 @@ Status parseQuery(boost::intrusive_ptr<ExpressionContext> expCtx, ParsedUpdate&
*/
void parseUpdate(boost::intrusive_ptr<ExpressionContext> expCtx, ParsedUpdate& parsedUpdate) {
parsedUpdate.driver->setCollator(expCtx->getCollator());
parsedUpdate.driver->setLogOp(true);
parsedUpdate.driver->setFromOplogApplication(parsedUpdate.request->isFromOplogApplication());
auto source = parsedUpdate.request->source();

8
src/mongo/db/update/delta_executor.cpp
View File

@ -33,6 +33,7 @@
#include "mongo/db/exec/mutable_bson/element.h"
#include "mongo/db/update/document_diff_applier.h"
#include "mongo/db/update/object_replace_executor.h"
#include "mongo/db/update/update_oplog_entry_serialization.h"
namespace mongo {
@ -47,6 +48,13 @@ DeltaExecutor::ApplyResult DeltaExecutor::applyUpdate(
auto result = ObjectReplaceExecutor::applyReplacementUpdate(
std::move(applyParams), postImage, postImageHasId);
result.oplogEntry = _outputOplogEntry;
// We could directly return the '_diff' object, but we would not be able to make any guarantees
// about its lifetime to callers.
if (auto diff = _outputOplogEntry[update_oplog_entry::kDiffObjectFieldName];
diff.isABSONObj()) {
result.diff = diff.embeddedObject();
}
return result;
}
} // namespace mongo

53
src/mongo/db/update/delta_executor_test.cpp
View File

@ -41,6 +41,8 @@
#include <vector>
#include <boost/optional/optional.hpp>
#define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kTest
@ -56,9 +58,8 @@ namespace {
* entry in the indexData vector, a return value of 3 (1+2) indicates that both first and second
* entry in the idnexDaa vector are affected by the modification.
*/
unsigned long getIndexAffectedFromLogEntry(std::vector<const UpdateIndexData*> indexData,
BSONObj logEntry) {
auto diff = update_oplog_entry::extractDiffFromOplogEntry(logEntry);
unsigned long getIndexAffectedFromDiff(std::vector<const UpdateIndexData*> indexData,
boost::optional<BSONObj> diff) {
if (!diff) {
return (unsigned long)-1;
}
@ -94,7 +95,7 @@ TEST(DeltaExecutorTest, Delete) {
mustCheckExistenceForInsertOperations);
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(), BSONObj());
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
@ -106,7 +107,7 @@ TEST(DeltaExecutorTest, Delete) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When the index path is a prefix of a path in the diff.
@ -117,7 +118,7 @@ TEST(DeltaExecutorTest, Delete) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {}, c: 1}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// With common parent, but path diverges.
@ -128,7 +129,7 @@ TEST(DeltaExecutorTest, Delete) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {c: 1}}}}"));
ASSERT_EQ(0, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(0, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
}
@ -148,7 +149,7 @@ TEST(DeltaExecutorTest, Update) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: false, f2: false, f3: false}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When a path in the diff is same as index path.
@ -159,7 +160,7 @@ TEST(DeltaExecutorTest, Update) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: false, c: false, p: false}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When the index path is a prefix of a path in the diff.
@ -170,7 +171,7 @@ TEST(DeltaExecutorTest, Update) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {c: false}, c: 1}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// With common parent, but path diverges.
@ -181,7 +182,7 @@ TEST(DeltaExecutorTest, Update) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {c: 1}, c: false}}}"));
ASSERT_EQ(0, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(0, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
}
@ -202,7 +203,7 @@ TEST(DeltaExecutorTest, Insert) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: false, f2: false, f3: false}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When a path in the diff is same as index path.
@ -213,7 +214,7 @@ TEST(DeltaExecutorTest, Insert) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {p: false, c: false, b: false}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When the index path is a prefix of a path in the diff.
@ -224,7 +225,7 @@ TEST(DeltaExecutorTest, Insert) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {c: {e: 1, d: 2}}}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// With common parent, but path diverges.
@ -235,7 +236,7 @@ TEST(DeltaExecutorTest, Insert) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: {c: 1}, c: 2}}}"));
ASSERT_EQ(0, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(0, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
}
@ -256,7 +257,7 @@ TEST(DeltaExecutorTest, InsertNumericFieldNamesTopLevel) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{'0': false, '1': false, '2': false}"));
ASSERT_EQ(1, getIndexAffectedFromLogEntry({&indexData}, result.oplogEntry));
ASSERT_EQ(1, getIndexAffectedFromDiff({&indexData}, result.diff));
}
{
auto doc = mutablebson::Document(preImage);
@ -266,7 +267,7 @@ TEST(DeltaExecutorTest, InsertNumericFieldNamesTopLevel) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{'0': false, '2': false}"));
ASSERT_EQ(0, getIndexAffectedFromLogEntry({&indexData}, result.oplogEntry));
ASSERT_EQ(0, getIndexAffectedFromDiff({&indexData}, result.diff));
}
}
@ -287,7 +288,7 @@ TEST(DeltaExecutorTest, InsertNumericFieldNamesNested) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{a: {'0': false, '1': false, '2': false}}"));
ASSERT_EQ(1, getIndexAffectedFromLogEntry({&indexData}, result.oplogEntry));
ASSERT_EQ(1, getIndexAffectedFromDiff({&indexData}, result.diff));
}
{
auto doc = mutablebson::Document(preImage);
@ -297,7 +298,7 @@ TEST(DeltaExecutorTest, InsertNumericFieldNamesNested) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{a: {'0': false, '2': false}}"));
ASSERT_EQ(1, getIndexAffectedFromLogEntry({&indexData}, result.oplogEntry));
ASSERT_EQ(1, getIndexAffectedFromDiff({&indexData}, result.diff));
}
}
@ -318,7 +319,7 @@ TEST(DeltaExecutorTest, ArraysInIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: [{a: {b: {c: 1}, c: 1}}]}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When the index path is a prefix of a path in the diff and also involves numeric
@ -330,7 +331,7 @@ TEST(DeltaExecutorTest, ArraysInIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: [{a: {b: {c: 1, d: 1}, c: 1}}, 1]}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When inserting a sub-object into array, and the sub-object diverges from the index path.
@ -341,7 +342,7 @@ TEST(DeltaExecutorTest, ArraysInIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: [{a: {b: {c: 1}, c: 1}}, 1, {b:1}]}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// When a common array path element is updated, but the paths diverge at the last element.
@ -352,7 +353,7 @@ TEST(DeltaExecutorTest, ArraysInIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: [{a: {b: {c: 1}}}, 1]}"));
ASSERT_EQ(0, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(0, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
}
@ -374,7 +375,7 @@ TEST(DeltaExecutorTest, ArraysAfterIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: [{c: 1}]}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// Updating a sub-array element.
@ -385,7 +386,7 @@ TEST(DeltaExecutorTest, ArraysAfterIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: [{c: 2}, 2]}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
{
// Updating an array element.
@ -396,7 +397,7 @@ TEST(DeltaExecutorTest, ArraysAfterIndexPath) {
auto result = test.applyUpdate(params);
ASSERT_BSONOBJ_BINARY_EQ(params.element.getDocument().getObject(),
fromjson("{f1: {a: {b: [1, 2]}}}"));
ASSERT_EQ(2, getIndexAffectedFromLogEntry({&indexData1, &indexData2}, result.oplogEntry));
ASSERT_EQ(2, getIndexAffectedFromDiff({&indexData1, &indexData2}, result.diff));
}
}

9
src/mongo/db/update/pipeline_executor.cpp
View File

@ -134,10 +134,13 @@ UpdateExecutor::ApplyResult PipelineExecutor::applyUpdate(ApplyParams applyParam
if (applyParams.logMode == ApplyParams::LogMode::kGenerateOplogEntry) {
// We're allowed to generate $v: 2 log entries. The $v:2 has certain meta-fields like
// '$v', 'diff'. So we pad some additional byte while computing diff.
const auto diff = doc_diff::computeOplogDiff(
originalDoc, transformedDoc, update_oplog_entry::kSizeOfDeltaOplogEntryMetadata);
if (diff) {
if (auto diff =
doc_diff::computeOplogDiff(originalDoc,
transformedDoc,
update_oplog_entry::kSizeOfDeltaOplogEntryMetadata);
diff) {
ret.oplogEntry = update_oplog_entry::makeDeltaOplogEntry(*diff);
ret.diff = std::move(diff);
return ret;
}
}

9
src/mongo/db/update/update_driver.cpp
View File

@ -275,7 +275,7 @@ Status UpdateDriver::update(OperationContext* opCtx,
bool validateForStorage,
const FieldRefSet& immutablePaths,
bool isInsert,
BSONObj* logOpRec,
DocumentUpdateRecord* updateRecord,
bool* docWasModified,
FieldRefSetWithStorage* modifiedPaths) {
// TODO SERVER-123161: assert that update() is called at most once in a !_multi case.
@ -297,7 +297,7 @@ Status UpdateDriver::update(OperationContext* opCtx,
applyParams.validateForStorage = false;
}
if (_logOp && logOpRec) {
if (updateRecord) {
applyParams.logMode = ApplyParams::LogMode::kGenerateOplogEntry;
if (MONGO_unlikely(hangAfterPipelineUpdateFCVCheck.shouldFail()) &&
@ -313,8 +313,9 @@ Status UpdateDriver::update(OperationContext* opCtx,
*docWasModified = !applyResult.noop;
}
if (_logOp && logOpRec && !applyResult.noop) {
*logOpRec = applyResult.oplogEntry;
if (updateRecord && !applyResult.noop) {
updateRecord->oplogEntry = applyResult.oplogEntry;
updateRecord->diff = applyResult.diff;
}
_containsDotsAndDollarsField =

27
src/mongo/db/update/update_driver.h
View File

@ -70,6 +70,14 @@ namespace mongo {
class CollatorInterface;
class OperationContext;
/**
* Holds the oplog entry and (for delta-style updates) the diff generated by UpdateDriver::update().
*/
struct DocumentUpdateRecord {
BSONObj oplogEntry;
boost::optional<BSONObj> diff;
};
class MONGO_MOD_PUBLIC UpdateDriver {
public:
enum class UpdateType { kOperator, kReplacement, kPipeline, kDelta, kTransform };
@ -113,9 +121,10 @@ public:
* the array item matched). If 'doc' allows the modifiers to be applied in place and no index
* updating is involved, then the modifiers may be applied "in place" over 'doc'.
*
* If the driver's '_logOp' mode is turned on, and if 'logOpRec' is not null, fills in the
* latter with the oplog entry corresponding to the update. If the modifiers can't be applied,
* returns an error status or uasserts with a corresponding description.
* If 'updateRecord' is not null, its 'oplogEntry' member is populated with the oplog entry
* corresponding to the update, and its 'diff' member is populated with the diff for delta-style
* updates. If the modifiers can't be applied, returns an error status or uasserts with a
* corresponding description.
*
* If 'validateForStorage' is true, ensures that modified elements do not violate depth or DBRef
* constraints. Ensures that no paths in 'immutablePaths' are modified (though they may be
@ -137,7 +146,7 @@ public:
bool validateForStorage,
const FieldRefSet& immutablePaths,
bool isInsert,
BSONObj* logOpRec = nullptr,
DocumentUpdateRecord* updateRecord = nullptr,
bool* docWasModified = nullptr,
FieldRefSetWithStorage* modifiedPaths = nullptr);
@ -167,13 +176,6 @@ public:
return _updateType;
}
bool logOp() const {
return _logOp;
}
void setLogOp(bool logOp) {
_logOp = logOp;
}
bool fromOplogApplication() const {
return _fromOplogApplication;
}
@ -247,9 +249,6 @@ private:
// mutable properties after parsing
//
// Should this driver generate an oplog record when it applies the update?
bool _logOp = false;
// True if this update comes from an oplog application.
bool _fromOplogApplication = false;

12
src/mongo/db/update/update_executor.h
View File

@ -29,6 +29,7 @@
#pragma once
#include "mongo/bson/bsonobj.h"
#include "mongo/db/exec/document_value/value.h"
#include "mongo/db/exec/mutable_bson/element.h"
#include "mongo/db/field_ref_set.h"
@ -36,6 +37,8 @@
#include "mongo/db/update_index_data.h"
#include "mongo/util/modules.h"
#include <boost/optional/optional.hpp>
namespace mongo {
class CollatorInterface;
@ -117,7 +120,16 @@ public:
// The oplog entry to log. This is only populated if the operation is not considered a
// noop and if the 'logMode' provided in ApplyParams indicates that an oplog entry should
// be generated.
//
// When populated, 'oplogEntry' is owned BSON.
BSONObj oplogEntry;
// The diff used to produce the oplog entry, if the oplog entry is a $v:2 delta entry.
// Populated whenever oplogEntry is a delta entry.
//
// NOTE: 'diff' may be a view into BSON owned by 'oplogEntry' and should only be treated as
// valid so long as 'oplogEntry' is valid.
boost::optional<BSONObj> diff;
};

7
src/mongo/db/update/update_node_test_fixture.h
View File

@ -29,6 +29,7 @@
#pragma once
#include "mongo/bson/bsontypes.h"
#include "mongo/bson/json.h"
#include "mongo/db/service_context_test_fixture.h"
#include "mongo/db/update/document_diff_calculator.h"
@ -95,14 +96,14 @@ protected:
if (!_indexData) {
return false;
}
auto diff = update_oplog_entry::extractDiffFromOplogEntry(logEntry);
if (!diff) {
auto diffElem = logEntry[update_oplog_entry::kDiffObjectFieldName];
if (diffElem.type() != BSONType::object) {
return false;
}
mongo::doc_diff::IndexUpdateIdentifier updateIdentifier{1 /*numIndexes*/};
updateIdentifier.addIndex(0 /*indexCounter*/, *_indexData);
return updateIdentifier.determineAffectedIndexes(*diff).any();
return updateIdentifier.determineAffectedIndexes(diffElem.embeddedObject()).any();
}
bool getIndexAffectedFromLogEntry() {

12
src/mongo/db/update/update_oplog_entry_serialization.cpp
View File

@ -79,18 +79,6 @@ BSONObj makeReplacementOplogEntry(const BSONObj& replacement) {
return builder.obj();
}
boost::optional<BSONObj> extractDiffFromOplogEntry(const BSONObj& opLog) {
auto version = opLog["$v"];
if (version.ok() &&
version.numberInt() == static_cast<int>(UpdateOplogEntryVersion::kDeltaV2)) {
auto diff = opLog[kDiffObjectFieldName];
if (diff.type() == BSONType::object) {
return diff.embeddedObject();
}
}
return {};
}
namespace {
BSONElement extractNewValueForFieldFromV2Entry(const BSONObj& oField, StringData fieldName) {
auto diffField = oField[kDiffObjectFieldName];

5
src/mongo/db/update/update_oplog_entry_serialization.h
View File

@ -73,11 +73,6 @@ enum class FieldRemovedStatus { kFieldRemoved, kFieldNotRemoved, kUnknown };
*/
BSONObj makeDeltaOplogEntry(const doc_diff::Diff& diff);
/**
* Given a $v: 2 delta-style oplog entry, return the embedded diff object.
*/
boost::optional<BSONObj> extractDiffFromOplogEntry(const BSONObj& opLog);
/**
* Produce the contents of the 'o' field of a replacement style oplog entry.
*

6
src/mongo/db/update/update_tree_executor.h
View File

@ -29,9 +29,11 @@
#pragma once
#include "mongo/bson/bsontypes.h"
#include "mongo/db/update/update_executor.h"
#include "mongo/db/update/update_node.h"
#include "mongo/db/update/update_object_node.h"
#include "mongo/db/update/update_oplog_entry_serialization.h"
#include "mongo/db/update/v2_log_builder.h"
#include "mongo/util/modules.h"
@ -58,6 +60,10 @@ public:
invariant(ret.oplogEntry.isEmpty());
if (auto logBuilder = updateNodeApplyParams.logBuilder) {
ret.oplogEntry = logBuilder->serialize();
if (auto diff = ret.oplogEntry[update_oplog_entry::kDiffObjectFieldName];
diff.isABSONObj()) {
ret.diff = diff.embeddedObject();
}
}
return ret;

14
src/mongo/db/update/update_util.cpp
View File

@ -57,7 +57,6 @@
#include "mongo/db/shard_role/shard_catalog/document_validation.h"
#include "mongo/db/shard_role/shard_catalog/operation_sharding_state.h"
#include "mongo/db/sharding_environment/grid.h"
#include "mongo/db/update/update_oplog_entry_serialization.h"
#include "mongo/logv2/log.h"
#include "mongo/s/resharding/resharding_feature_flag_gen.h"
#include "mongo/s/would_change_owning_shard_exception.h"
@ -124,7 +123,6 @@ void generateNewDocumentFromSuppliedDoc(OperationContext* opCtx,
replacementDriver.parse(
write_ops::UpdateModification(suppliedDoc, write_ops::UpdateModification::ReplacementTag{}),
{});
replacementDriver.setLogOp(false);
replacementDriver.setBypassEmptyTsReplacement(
static_cast<bool>(request->getBypassEmptyTsReplacement()));
@ -431,7 +429,7 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
matchedField = matchDetails.elemMatchKey();
}
BSONObj logObj;
DocumentUpdateRecord updateRecord;
bool docWasModified = false;
status = driver->update(opCtx,
matchedField,
@ -439,7 +437,7 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
isUserInitiatedWrite,
immutablePaths,
false, /* isInsert */
&logObj,
&updateRecord,
&docWasModified);
uassertStatusOK(status);
@ -470,7 +468,7 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
// Prepare to modify the document
CollectionUpdateArgs args{oldObjValue};
args.update = logObj;
args.update = updateRecord.oplogEntry;
if (isUserInitiatedWrite) {
const auto& collDesc = collection.getShardingDescription();
args.criteria = collDesc.extractDocumentKey(oldObj.value());
@ -512,7 +510,6 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
scfu.checkUpdateChangesShardKeyFields(opCtx, doc, boost::none /* newObj */, oldObj);
}
auto diff = update_oplog_entry::extractDiffFromOplogEntry(logObj);
WriteUnitOfWork wunit(opCtx);
newObj = uassertStatusOK(collection_internal::updateDocumentWithDamages(
opCtx,
@ -521,7 +518,8 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
oldObj,
source,
damages,
diff.has_value() ? &*diff : collection_internal::kUpdateAllIndexes,
updateRecord.diff.has_value() ? &*updateRecord.diff
: collection_internal::kUpdateAllIndexes,
&indexesAffected,
&CurOp::get(opCtx)->debug(),
&args,
@ -555,7 +553,7 @@ std::pair<BSONObj, bool> transformDocument(OperationContext* opCtx,
scfu.checkUpdateChangesShardKeyFields(opCtx, doc, newObj, oldObj);
}
auto diff = update_oplog_entry::extractDiffFromOplogEntry(logObj);
const auto& diff = updateRecord.diff;
WriteUnitOfWork wunit(opCtx);
collection_internal::updateDocument(
opCtx,

4
src/mongo/embedded/stitch_support/stitch_support.cpp
View File

@ -628,7 +628,7 @@ stitch_support_v1_update_apply(stitch_support_v1_update* const update,
false /* validateForStorage */,
immutablePaths,
false /* isInsert */,
nullptr /* logOpRec*/,
nullptr /* updateRecord */,
&docWasModified,
&modifiedPaths));
@ -675,7 +675,7 @@ uint8_t* MONGO_API_CALL stitch_support_v1_update_upsert(stitch_support_v1_update
false /* validateForStorage */,
immutablePaths,
true /* isInsert */,
nullptr /* logOpRec */,
nullptr /* updateRecord */,
&docWasModified,
nullptr /* modifiedPaths */));