-
Type: Bug
-
Resolution: Duplicate
-
Priority: Major - P3
-
None
-
Affects Version/s: None
-
Component/s: None
-
Labels:None
-
ALL
-
Hide
Run resmoke.py --suite=replica_sets jstests/replsets/apply_ops_idempotency.js with the following patch:
Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yamldiff --git a/src/mongo/db/catalog/index_timestamp_helper.cpp b/src/mongo/db/catalog/index_timestamp_helper.cpp index ceecf0b838..8686e7af8f 100644 --- a/src/mongo/db/catalog/index_timestamp_helper.cpp +++ b/src/mongo/db/catalog/index_timestamp_helper.cpp @@ -102,13 +102,12 @@ void IndexTimestampHelper::setGhostCommitTimestampForWrite(OperationContext* opC fassert(51053, status); } -/** - * Returns true if writes to the catalog entry for the input namespace require being - * timestamped. A ghost write is when the operation is not committed with an oplog entry and - * implies the caller will look at the logical clock to choose a time to use. - */ -namespace { -bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, NamespaceString nss) { +bool IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, + NamespaceString nss) { + if (opCtx->writesAreReplicated()) { + return false; + } + if (!nss.isReplicated() || nss.coll().startsWith("tmp.mr.")) { return false; } @@ -144,7 +143,6 @@ bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, Namesp return true; } -} // namespace bool IndexTimestampHelper::setGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, const NamespaceString& nss) { diff --git a/src/mongo/db/catalog/index_timestamp_helper.h b/src/mongo/db/catalog/index_timestamp_helper.h index 9ae4457e40..7953c8ef67 100644 --- a/src/mongo/db/catalog/index_timestamp_helper.h +++ b/src/mongo/db/catalog/index_timestamp_helper.h @@ -36,6 +36,13 @@ namespace mongo { namespace IndexTimestampHelper { +/** + * Returns true if writes to the catalog entry for the input namespace require being + * timestamped. A ghost write is when the operation is not committed with an oplog entry and + * implies the caller will look at the logical clock to choose a time to use. + */ +bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, NamespaceString nss); + /** * If required, sets a timestamp on an active WriteUnitOfWork. A ghost write is when the * operation is not committed with an oplog entry, which may be necessary for certain index diff --git a/src/mongo/db/storage/SConscript b/src/mongo/db/storage/SConscript index bff1205b15..af58c0c262 100644 --- a/src/mongo/db/storage/SConscript +++ b/src/mongo/db/storage/SConscript @@ -435,7 +435,8 @@ env.Library( ], LIBDEPS_PRIVATE=[ '$BUILD_DIR/mongo/db/catalog/disable_index_spec_namespace_generation', - ] + '$BUILD_DIR/mongo/db/catalog/index_timestamp_helper', + ], ) env.Library( diff --git a/src/mongo/db/storage/durable_catalog_impl.cpp b/src/mongo/db/storage/durable_catalog_impl.cpp index 8e3d75c664..9997da0ab9 100644 --- a/src/mongo/db/storage/durable_catalog_impl.cpp +++ b/src/mongo/db/storage/durable_catalog_impl.cpp @@ -36,6 +36,7 @@ #include "mongo/bson/util/bson_extract.h" #include "mongo/bson/util/builder.h" #include "mongo/db/catalog/disable_index_spec_namespace_generation_gen.h" +#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/namespace_string.h" @@ -602,6 +603,11 @@ void DurableCatalogImpl::putMetaData(OperationContext* opCtx, obj = b.obj(); } + if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, nss) && + !nss.isDropPendingNamespace()) { + opCtx->recoveryUnit()->setMustBeTimestamped(); + } + LOG(3) << "recording new metadata: " << obj; Status status = _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize()); fassert(28521, status.isOK()); @@ -643,6 +649,11 @@ Status DurableCatalogImpl::_replaceEntry(OperationContext* opCtx, _idents.erase(fromIt); _idents[toNss.toString()] = Entry(old["ident"].String(), loc); + if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, fromNss) && + !fromNss.isDropPendingNamespace()) { + opCtx->recoveryUnit()->setMustBeTimestamped(); + } + return Status::OK(); } diff --git a/src/mongo/db/storage/recovery_unit.h b/src/mongo/db/storage/recovery_unit.h index cd640c82e2..91c6306769 100644 --- a/src/mongo/db/storage/recovery_unit.h +++ b/src/mongo/db/storage/recovery_unit.h @@ -509,8 +509,14 @@ public: virtual void setOrderedCommit(bool orderedCommit) = 0; + void setMustBeTimestamped() { + _mustBeTimestamped = true; + } + protected: RecoveryUnit() {} + + bool _mustBeTimestamped = false; }; } // namespace mongo diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp index ea78f02b27..26be848b02 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp @@ -397,6 +397,10 @@ void WiredTigerRecoveryUnit::_txnClose(bool commit) { conf << "durable_timestamp=" << integerToHex(_durableTimestamp.asULL()); } + if (_mustBeTimestamped) { + invariant(_isTimestamped); + } + wtRet = s->commit_transaction(s, conf.str().c_str()); LOG(3) << "WT commit_transaction for snapshot id " << _mySnapshotId; } else { @@ -435,6 +439,7 @@ void WiredTigerRecoveryUnit::_txnClose(bool commit) { _isOplogReader = false; _oplogVisibleTs = boost::none; _orderedCommit = true; // Default value is true; we assume all writes are ordered. + _mustBeTimestamped = false; } SnapshotId WiredTigerRecoveryUnit::getSnapshotId() const { diff --git a/src/mongo/dbtests/dbtests.cpp b/src/mongo/dbtests/dbtests.cpp index 3a16b947f9..446906a7ed 100644 --- a/src/mongo/dbtests/dbtests.cpp +++ b/src/mongo/dbtests/dbtests.cpp @@ -106,7 +106,17 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj } MultiIndexBlock indexer; ON_BLOCK_EXIT([&] { indexer.cleanUpAfterBuild(opCtx, coll); }); - Status status = indexer.init(opCtx, coll, spec, MultiIndexBlock::kNoopOnInitFn).getStatus(); + Status status = indexer + .init(opCtx, + coll, + spec, + [opCtx](const std::vector<BSONObj>& specs) -> Status { + if (opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { + return opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1)); + } + return Status::OK(); + }) + .getStatus(); if (status == ErrorCodes::IndexAlreadyExists) { return Status::OK(); } @@ -124,6 +134,7 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj WriteUnitOfWork wunit(opCtx); ASSERT_OK(indexer.commit( opCtx, coll, MultiIndexBlock::kNoopOnCreateEachFn, MultiIndexBlock::kNoopOnCommitFn)); + ASSERT_OK(opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1))); wunit.commit(); return Status::OK(); } diff --git a/src/mongo/dbtests/repltests.cpp b/src/mongo/dbtests/repltests.cpp index ccb1e796b2..de0a6c1490 100644 --- a/src/mongo/dbtests/repltests.cpp +++ b/src/mongo/dbtests/repltests.cpp @@ -145,6 +145,8 @@ public: ASSERT(c->getIndexCatalog()->haveIdIndex(&_opCtx)); wuow.commit(); + + _opCtx.getServiceContext()->getStorageEngine()->setOldestTimestamp(_lastSetOldestTimestamp); } ~Base() { // Replication is not supported by mobile SE. @@ -239,6 +241,8 @@ protected: if (auto tsElem = ops.front()["ts"]) { _opCtx.getServiceContext()->getStorageEngine()->setOldestTimestamp( tsElem.timestamp()); + _lastSetOldestTimestamp = + std::max(_lastSetOldestTimestamp, tsElem.timestamp()); } } } @@ -305,6 +309,7 @@ protected: repl::UnreplicatedWritesBlock uwb(&_opCtx); coll->insertDocument(&_opCtx, InsertStatement(o), nullOpDebug, true) .transitional_ignore(); + ASSERT_OK(_opCtx.recoveryUnit()->setTimestamp(_lastSetOldestTimestamp)); wunit.commit(); return; } @@ -317,6 +322,7 @@ protected: repl::UnreplicatedWritesBlock uwb(&_opCtx); coll->insertDocument(&_opCtx, InsertStatement(b.obj()), nullOpDebug, true) .transitional_ignore(); + ASSERT_OK(_opCtx.recoveryUnit()->setTimestamp(_lastSetOldestTimestamp)); wunit.commit(); } static BSONObj wid(const char* json) { @@ -327,6 +333,8 @@ protected: b.appendElements(fromjson(json)); return b.obj(); } + + Timestamp _lastSetOldestTimestamp = Timestamp(1, 1); }; diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp index 8bfacc5a18..8f7e85c1ee 100644 --- a/src/mongo/dbtests/storage_timestamp_tests.cpp +++ b/src/mongo/dbtests/storage_timestamp_tests.cpp @@ -241,6 +241,9 @@ public: AutoGetOrCreateDb dbRaii(_opCtx, nss.db(), LockMode::MODE_X); WriteUnitOfWork wunit(_opCtx); + if (_opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { + ASSERT_OK(_opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1))); + } invariant(dbRaii.getDb()->createCollection(_opCtx, nss)); wunit.commit(); });
ShowRun resmoke.py --suite=replica_sets jstests/replsets/apply_ops_idempotency.js with the following patch: Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml diff --git a/src/mongo/db/catalog/index_timestamp_helper.cpp b/src/mongo/db/catalog/index_timestamp_helper.cpp index ceecf0b838..8686e7af8f 100644 --- a/src/mongo/db/catalog/index_timestamp_helper.cpp +++ b/src/mongo/db/catalog/index_timestamp_helper.cpp @@ -102,13 +102,12 @@ void IndexTimestampHelper::setGhostCommitTimestampForWrite(OperationContext* opC fassert(51053, status); } -/** - * Returns true if writes to the catalog entry for the input namespace require being - * timestamped. A ghost write is when the operation is not committed with an oplog entry and - * implies the caller will look at the logical clock to choose a time to use. - */ -namespace { -bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, NamespaceString nss) { +bool IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, + NamespaceString nss) { + if (opCtx->writesAreReplicated()) { + return false ; + } + if (!nss.isReplicated() || nss.coll().startsWith( "tmp.mr." )) { return false ; } @@ -144,7 +143,6 @@ bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, Namesp return true ; } -} // namespace bool IndexTimestampHelper::setGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, const NamespaceString& nss) { diff --git a/src/mongo/db/catalog/index_timestamp_helper.h b/src/mongo/db/catalog/index_timestamp_helper.h index 9ae4457e40..7953c8ef67 100644 --- a/src/mongo/db/catalog/index_timestamp_helper.h +++ b/src/mongo/db/catalog/index_timestamp_helper.h @@ -36,6 +36,13 @@ namespace mongo { namespace IndexTimestampHelper { +/** + * Returns true if writes to the catalog entry for the input namespace require being + * timestamped. A ghost write is when the operation is not committed with an oplog entry and + * implies the caller will look at the logical clock to choose a time to use. + */ +bool requiresGhostCommitTimestampForCatalogWrite(OperationContext* opCtx, NamespaceString nss); + /** * If required, sets a timestamp on an active WriteUnitOfWork. A ghost write is when the * operation is not committed with an oplog entry, which may be necessary for certain index diff --git a/src/mongo/db/storage/SConscript b/src/mongo/db/storage/SConscript index bff1205b15..af58c0c262 100644 --- a/src/mongo/db/storage/SConscript +++ b/src/mongo/db/storage/SConscript @@ -435,7 +435,8 @@ env.Library( ], LIBDEPS_PRIVATE=[ '$BUILD_DIR/mongo/db/catalog/disable_index_spec_namespace_generation' , - ] + '$BUILD_DIR/mongo/db/catalog/index_timestamp_helper' , + ], ) env.Library( diff --git a/src/mongo/db/storage/durable_catalog_impl.cpp b/src/mongo/db/storage/durable_catalog_impl.cpp index 8e3d75c664..9997da0ab9 100644 --- a/src/mongo/db/storage/durable_catalog_impl.cpp +++ b/src/mongo/db/storage/durable_catalog_impl.cpp @@ -36,6 +36,7 @@ #include "mongo/bson/util/bson_extract.h" #include "mongo/bson/util/builder.h" #include "mongo/db/catalog/disable_index_spec_namespace_generation_gen.h" +#include "mongo/db/catalog/index_timestamp_helper.h" #include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/namespace_string.h" @@ -602,6 +603,11 @@ void DurableCatalogImpl::putMetaData(OperationContext* opCtx, obj = b.obj(); } + if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, nss) && + !nss.isDropPendingNamespace()) { + opCtx->recoveryUnit()->setMustBeTimestamped(); + } + LOG(3) << "recording new metadata: " << obj; Status status = _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize()); fassert(28521, status.isOK()); @@ -643,6 +649,11 @@ Status DurableCatalogImpl::_replaceEntry(OperationContext* opCtx, _idents.erase(fromIt); _idents[toNss.toString()] = Entry(old[ "ident" ]. String (), loc); + if (IndexTimestampHelper::requiresGhostCommitTimestampForCatalogWrite(opCtx, fromNss) && + !fromNss.isDropPendingNamespace()) { + opCtx->recoveryUnit()->setMustBeTimestamped(); + } + return Status::OK(); } diff --git a/src/mongo/db/storage/recovery_unit.h b/src/mongo/db/storage/recovery_unit.h index cd640c82e2..91c6306769 100644 --- a/src/mongo/db/storage/recovery_unit.h +++ b/src/mongo/db/storage/recovery_unit.h @@ -509,8 +509,14 @@ public : virtual void setOrderedCommit(bool orderedCommit) = 0; + void setMustBeTimestamped() { + _mustBeTimestamped = true ; + } + protected : RecoveryUnit() {} + + bool _mustBeTimestamped = false ; }; } // namespace mongo diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp index ea78f02b27..26be848b02 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp @@ -397,6 +397,10 @@ void WiredTigerRecoveryUnit::_txnClose(bool commit) { conf << "durable_timestamp=" << integerToHex(_durableTimestamp.asULL()); } + if (_mustBeTimestamped) { + invariant(_isTimestamped); + } + wtRet = s->commit_transaction(s, conf.str().c_str()); LOG(3) << "WT commit_transaction for snapshot id " << _mySnapshotId; } else { @@ -435,6 +439,7 @@ void WiredTigerRecoveryUnit::_txnClose(bool commit) { _isOplogReader = false ; _oplogVisibleTs = boost::none; _orderedCommit = true ; // Default value is true ; we assume all writes are ordered. + _mustBeTimestamped = false ; } SnapshotId WiredTigerRecoveryUnit::getSnapshotId() const { diff --git a/src/mongo/dbtests/dbtests.cpp b/src/mongo/dbtests/dbtests.cpp index 3a16b947f9..446906a7ed 100644 --- a/src/mongo/dbtests/dbtests.cpp +++ b/src/mongo/dbtests/dbtests.cpp @@ -106,7 +106,17 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj } MultiIndexBlock indexer; ON_BLOCK_EXIT([&] { indexer.cleanUpAfterBuild(opCtx, coll); }); - Status status = indexer.init(opCtx, coll, spec, MultiIndexBlock::kNoopOnInitFn).getStatus(); + Status status = indexer + .init(opCtx, + coll, + spec, + [opCtx]( const std::vector<BSONObj>& specs) -> Status { + if (opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { + return opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1)); + } + return Status::OK(); + }) + .getStatus(); if (status == ErrorCodes::IndexAlreadyExists) { return Status::OK(); } @@ -124,6 +134,7 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj WriteUnitOfWork wunit(opCtx); ASSERT_OK(indexer.commit( opCtx, coll, MultiIndexBlock::kNoopOnCreateEachFn, MultiIndexBlock::kNoopOnCommitFn)); + ASSERT_OK(opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1))); wunit.commit(); return Status::OK(); } diff --git a/src/mongo/dbtests/repltests.cpp b/src/mongo/dbtests/repltests.cpp index ccb1e796b2..de0a6c1490 100644 --- a/src/mongo/dbtests/repltests.cpp +++ b/src/mongo/dbtests/repltests.cpp @@ -145,6 +145,8 @@ public : ASSERT(c->getIndexCatalog()->haveIdIndex(&_opCtx)); wuow.commit(); + + _opCtx.getServiceContext()->getStorageEngine()->setOldestTimestamp(_lastSetOldestTimestamp); } ~Base() { // Replication is not supported by mobile SE. @@ -239,6 +241,8 @@ protected : if (auto tsElem = ops.front()[ "ts" ]) { _opCtx.getServiceContext()->getStorageEngine()->setOldestTimestamp( tsElem.timestamp()); + _lastSetOldestTimestamp = + std::max(_lastSetOldestTimestamp, tsElem.timestamp()); } } } @@ -305,6 +309,7 @@ protected : repl::UnreplicatedWritesBlock uwb(&_opCtx); coll->insertDocument(&_opCtx, InsertStatement(o), nullOpDebug, true ) .transitional_ignore(); + ASSERT_OK(_opCtx.recoveryUnit()->setTimestamp(_lastSetOldestTimestamp)); wunit.commit(); return ; } @@ -317,6 +322,7 @@ protected : repl::UnreplicatedWritesBlock uwb(&_opCtx); coll->insertDocument(&_opCtx, InsertStatement(b.obj()), nullOpDebug, true ) .transitional_ignore(); + ASSERT_OK(_opCtx.recoveryUnit()->setTimestamp(_lastSetOldestTimestamp)); wunit.commit(); } static BSONObj wid( const char * json) { @@ -327,6 +333,8 @@ protected : b.appendElements(fromjson(json)); return b.obj(); } + + Timestamp _lastSetOldestTimestamp = Timestamp(1, 1); }; diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp index 8bfacc5a18..8f7e85c1ee 100644 --- a/src/mongo/dbtests/storage_timestamp_tests.cpp +++ b/src/mongo/dbtests/storage_timestamp_tests.cpp @@ -241,6 +241,9 @@ public : AutoGetOrCreateDb dbRaii(_opCtx, nss.db(), LockMode::MODE_X); WriteUnitOfWork wunit(_opCtx); + if (_opCtx->recoveryUnit()->getCommitTimestamp().isNull()) { + ASSERT_OK(_opCtx->recoveryUnit()->setTimestamp(Timestamp(1, 1))); + } invariant(dbRaii.getDb()->createCollection(_opCtx, nss)); wunit.commit(); }); -
Execution Team 2020-02-24
-
39
I encountered this issue attempting to backport SERVER-42497 to 4.2. I am not certain of about the cause. The renameCollection logic is very similar between 4.2 and master.