Details
-
Bug
-
Resolution: Duplicate
-
Major - P3
-
None
-
None
-
None
-
None
-
ALL
-
Hide
Run resmoke.py --suite=replica_sets jstests/replsets/apply_ops_idempotency.js with the following patch:
diff --git a/src/mongo/db/catalog/index_timestamp_helper.cpp b/src/mongo/db/catalog/index_timestamp_helper.cppindex 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* opCfassert(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, Namespreturn true;}-} // namespacebool 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.hindex 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 indexdiff --git a/src/mongo/db/storage/SConscript b/src/mongo/db/storage/SConscriptindex 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.cppindex 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.hindex 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 mongodiff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit.cppindex 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.cppindex 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 BSONObjWriteUnitOfWork 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.cppindex 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.cppindex 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: 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
Description
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.