summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Gottlieb <daniel.gottlieb@mongodb.com>2019-10-17 15:10:46 +0000
committerevergreen <evergreen@mongodb.com>2019-10-17 15:10:46 +0000
commit65decc93b23acf373d33d7453bb2ba745a6ef319 (patch)
tree87b24d3bce0740afaeef10928456c689714fba20
parent1744f22d21339bcf212cb66d85cd4c582d123c0e (diff)
downloadmongo-65decc93b23acf373d33d7453bb2ba745a6ef319.tar.gz
SERVER-43858: Relax database creation lock acquisition from MODE_X to MODE_IX.
-rw-r--r--src/mongo/db/catalog/database_holder_impl.cpp17
-rw-r--r--src/mongo/db/catalog_raii.cpp12
-rw-r--r--src/mongo/db/catalog_raii.h5
-rw-r--r--src/mongo/db/db_raii.cpp1
-rw-r--r--src/mongo/db/repl/oplog_applier_impl_test.cpp9
-rw-r--r--src/mongo/dbtests/dbtests.cpp16
-rw-r--r--src/mongo/dbtests/dbtests.h4
7 files changed, 24 insertions, 40 deletions
diff --git a/src/mongo/db/catalog/database_holder_impl.cpp b/src/mongo/db/catalog/database_holder_impl.cpp
index c105b6bc07b..7f75cbffe48 100644
--- a/src/mongo/db/catalog/database_holder_impl.cpp
+++ b/src/mongo/db/catalog/database_holder_impl.cpp
@@ -109,7 +109,7 @@ std::vector<std::string> DatabaseHolderImpl::getNames() {
Database* DatabaseHolderImpl::openDb(OperationContext* opCtx, StringData ns, bool* justCreated) {
const StringData dbname = _todb(ns);
- invariant(opCtx->lockState()->isDbLockedForMode(dbname, MODE_X));
+ invariant(opCtx->lockState()->isDbLockedForMode(dbname, MODE_IX));
if (justCreated)
*justCreated = false; // Until proven otherwise.
@@ -136,11 +136,8 @@ Database* DatabaseHolderImpl::openDb(OperationContext* opCtx, StringData ns, boo
<< "]",
duplicates.empty());
-
// Do the catalog lookup and database creation outside of the scoped lock, because these may
- // block. Only one thread can be inside this method for the same DB name, because of the
- // requirement for X-lock on the database when we enter. So there is no way we can insert two
- // different databases for the same name.
+ // block.
lk.unlock();
if (CollectionCatalog::get(opCtx).getAllCollectionUUIDsFromDb(dbname).empty()) {
@@ -156,7 +153,15 @@ Database* DatabaseHolderImpl::openDb(OperationContext* opCtx, StringData ns, boo
removeDbGuard.dismiss();
lk.lock();
auto it = _dbs.find(dbname);
- invariant(it != _dbs.end() && it->second == nullptr);
+ // Dropping a database requires a MODE_X lock, so the entry in the `_dbs` map cannot disappear.
+ invariant(it != _dbs.end());
+ if (it->second) {
+ // Creating databases only requires a DB lock in MODE_IX. Thus databases can concurrently
+ // created. If this thread "lost the race", return the database object that was persisted in
+ // the `_dbs` map.
+ return it->second;
+ }
+
it->second = newDb.release();
invariant(_getNamesWithConflictingCasing_inlock(dbname.toString()).empty());
diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp
index 1a207786be2..41bf887d77b 100644
--- a/src/mongo/db/catalog_raii.cpp
+++ b/src/mongo/db/catalog_raii.cpp
@@ -135,18 +135,12 @@ AutoGetCollection::AutoGetCollection(OperationContext* opCtx,
AutoGetOrCreateDb::AutoGetOrCreateDb(OperationContext* opCtx,
StringData dbName,
LockMode mode,
- Date_t deadline) {
+ Date_t deadline)
+ : _autoDb(opCtx, dbName, mode, deadline) {
invariant(mode == MODE_IX || mode == MODE_X);
- _autoDb.emplace(opCtx, dbName, mode, deadline);
- _db = _autoDb->getDb();
-
- // If the database didn't exist, relock in MODE_X
+ _db = _autoDb.getDb();
if (!_db) {
- if (mode != MODE_X) {
- _autoDb.emplace(opCtx, dbName, MODE_X, deadline);
- }
-
auto databaseHolder = DatabaseHolder::get(opCtx);
_db = databaseHolder->openDb(opCtx, dbName, &_justCreated);
}
diff --git a/src/mongo/db/catalog_raii.h b/src/mongo/db/catalog_raii.h
index f3fcae654ef..4172a54514b 100644
--- a/src/mongo/db/catalog_raii.h
+++ b/src/mongo/db/catalog_raii.h
@@ -148,8 +148,7 @@ private:
* RAII-style class, which acquires a lock on the specified database in the requested mode and
* obtains a reference to the database, creating it was non-existing. Used as a shortcut for
* calls to DatabaseHolder::get(opCtx)->openDb(), taking care of locking details. The
- * requested mode must be MODE_IX or MODE_X. If the database needs to be created, the lock will
- * automatically be reacquired as MODE_X.
+ * requested mode must be MODE_IX or MODE_X.
*
* Use this when you are about to perform a write, and want to create the database if it doesn't
* already exist.
@@ -176,7 +175,7 @@ public:
}
private:
- boost::optional<AutoGetDb> _autoDb;
+ AutoGetDb _autoDb;
Database* _db;
bool _justCreated{false};
diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp
index cdb3813f985..ede268a31f3 100644
--- a/src/mongo/db/db_raii.cpp
+++ b/src/mongo/db/db_raii.cpp
@@ -329,7 +329,6 @@ OldClientContext::OldClientContext(OperationContext* opCtx, const std::string& n
: _opCtx(opCtx), _db(DatabaseHolder::get(opCtx)->getDb(opCtx, ns)) {
if (!_db) {
const auto dbName = nsToDatabaseSubstring(ns);
- invariant(_opCtx->lockState()->isDbLockedForMode(dbName, MODE_X));
_db = DatabaseHolder::get(opCtx)->openDb(_opCtx, dbName, &_justCreated);
invariant(_db);
}
diff --git a/src/mongo/db/repl/oplog_applier_impl_test.cpp b/src/mongo/db/repl/oplog_applier_impl_test.cpp
index 32df5b58f79..9338da454d6 100644
--- a/src/mongo/db/repl/oplog_applier_impl_test.cpp
+++ b/src/mongo/db/repl/oplog_applier_impl_test.cpp
@@ -117,7 +117,8 @@ void createCollection(OperationContext* opCtx,
const NamespaceString& nss,
const CollectionOptions& options) {
writeConflictRetry(opCtx, "createCollection", nss.ns(), [&] {
- Lock::DBLock dblk(opCtx, nss.db(), MODE_X);
+ Lock::DBLock dblk(opCtx, nss.db(), MODE_IX);
+ Lock::CollectionLock collLk(opCtx, nss, MODE_X);
OldClientContext ctx(opCtx, nss.ns());
auto db = ctx.db();
ASSERT_TRUE(db);
@@ -210,7 +211,7 @@ TEST_F(OplogApplierImplTest, applyOplogEntryOrGroupedInsertsInsertDocumentCollec
createDatabase(_opCtx.get(), nss.db());
// Even though the collection doesn't exist, this is handled in the actual application function,
// which in the case of this test just ignores such errors. This tests mostly that we don't
- // implicitly create the collection and lock the database in MODE_X.
+ // implicitly create the collection.
auto op = makeOplogEntry(OpTypeEnum::kInsert, nss, {});
ASSERT_THROWS(_applyOplogEntryOrGroupedInsertsWrapper(
_opCtx.get(), &op, OplogApplication::Mode::kSecondary),
@@ -223,7 +224,7 @@ TEST_F(OplogApplierImplTest, applyOplogEntryOrGroupedInsertsDeleteDocumentCollec
createDatabase(_opCtx.get(), nss.db());
// Even though the collection doesn't exist, this is handled in the actual application function,
// which in the case of this test just ignores such errors. This tests mostly that we don't
- // implicitly create the collection and lock the database in MODE_X.
+ // implicitly create the collection.
auto op = makeOplogEntry(OpTypeEnum::kDelete, nss, {});
_testApplyOplogEntryOrGroupedInsertsCrudOperation(ErrorCodes::OK, op, false);
ASSERT_FALSE(collectionExists(_opCtx.get(), nss));
@@ -279,7 +280,7 @@ TEST_F(OplogApplierImplTest, applyOplogEntryOrGroupedInsertsCommand) {
const BSONObj&) {
applyCmdCalled = true;
ASSERT_TRUE(opCtx);
- ASSERT_TRUE(opCtx->lockState()->isDbLockedForMode(nss.db(), MODE_X));
+ ASSERT_TRUE(opCtx->lockState()->isDbLockedForMode(nss.db(), MODE_IX));
ASSERT_EQUALS(nss, collNss);
return Status::OK();
};
diff --git a/src/mongo/dbtests/dbtests.cpp b/src/mongo/dbtests/dbtests.cpp
index 64902947f37..e094c2ed223 100644
--- a/src/mongo/dbtests/dbtests.cpp
+++ b/src/mongo/dbtests/dbtests.cpp
@@ -157,22 +157,8 @@ WriteContextForTests::WriteContextForTests(OperationContext* opCtx, StringData n
if (getCollection())
return;
- // If the database was just created, it is already locked in MODE_X so we can skip the relocking
- // code below
- if (_autoCreateDb->justCreated()) {
- dassert(opCtx->lockState()->isDbLockedForMode(_nss.db(), MODE_X));
- return;
- }
-
- // If the collection doesn't exists, put the context in a state where the database is locked in
- // MODE_X so that the collection can be created
- _clientContext.reset();
- _collLock.reset();
- _autoCreateDb.reset();
- _autoCreateDb.emplace(opCtx, _nss.db(), MODE_X);
-
- _clientContext.emplace(opCtx, _nss.ns(), _autoCreateDb->getDb());
invariant(_autoCreateDb->getDb() == _clientContext->db());
+ _collLock.emplace(opCtx, _nss, MODE_X);
}
} // namespace dbtests
diff --git a/src/mongo/dbtests/dbtests.h b/src/mongo/dbtests/dbtests.h
index f0b0166aecb..b40c46b8fb1 100644
--- a/src/mongo/dbtests/dbtests.h
+++ b/src/mongo/dbtests/dbtests.h
@@ -65,8 +65,8 @@ Status createIndexFromSpec(OperationContext* opCtx, StringData ns, const BSONObj
/**
* Combines AutoGetOrCreateDb and OldClientContext. If the requested 'ns' exists, the constructed
* object will have both the database and the collection locked in MODE_IX. Otherwise, the database
- * will be locked in MODE_X and will be created (note, only the database will be created, but not
- * the collection).
+ * will be locked in MODE_IX and will be created, while the collection will be locked in MODE_X, but
+ * not created.
*/
class WriteContextForTests {
WriteContextForTests(const WriteContextForTests&) = delete;