From c44a3ecdffd5c98fdd2daa6e46f52c50ff594576 Mon Sep 17 00:00:00 2001
From: Mathias Stearn <redbeard0531@gmail.com>
Date: Fri, 15 Jan 2016 16:30:52 -0500
Subject: [PATCH] SERVER-19128 Clean up interaction of background index builds
and cursor killing
This reverts commit 90a684ad25a86deff16f80e11e257c6ac6611683, restoring
1d26b77d115eb39f03dffbdbaccf10e696cd4fe3.
---
src/mongo/db/background.h | 10 ++++++++++
src/mongo/db/catalog/collection.cpp | 6 +++++-
src/mongo/db/catalog/cursor_manager.cpp | 2 ++
src/mongo/db/catalog/database.cpp | 2 ++
src/mongo/db/catalog/index_catalog.cpp | 17 ++++++++++++-----
src/mongo/db/catalog/index_create.cpp | 14 +++++---------
src/mongo/db/dbcommands.cpp | 4 ++++
7 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/src/mongo/db/background.h b/src/mongo/db/background.h
index b5ec4caea70..3a40a286bda 100644
--- a/src/mongo/db/background.h
+++ b/src/mongo/db/background.h
@@ -64,6 +64,16 @@ public:
static void awaitNoBgOpInProgForNs(const StringData& ns);
static void dump(std::ostream&);
+ static bool inProgForNs(const NamespaceString& ns) {
+ return inProgForNs(ns.ns());
+ }
+ static void assertNoBgOpInProgForNs(const NamespaceString& ns) {
+ assertNoBgOpInProgForNs(ns.ns());
+ }
+ static void awaitNoBgOpInProgForNs(const NamespaceString& ns) {
+ awaitNoBgOpInProgForNs(ns.ns());
+ }
+
/* check for in progress before instantiating */
BackgroundOperation(const StringData& ns);
diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
index 936909a4456..89e58c10021 100644
--- a/src/mongo/db/catalog/collection.cpp
+++ b/src/mongo/db/catalog/collection.cpp
@@ -38,6 +38,7 @@
#include "mongo/base/counter.h"
#include "mongo/base/owned_pointer_map.h"
+#include "mongo/db/background.h"
#include "mongo/db/clientcursor.h"
#include "mongo/db/commands/server_status_metric.h"
#include "mongo/db/curop.h"
@@ -480,7 +481,8 @@ uint64_t Collection::getIndexSize(OperationContext* opCtx, BSONObjBuilder* detai
*/
Status Collection::truncate(OperationContext* txn) {
dassert(txn->lockState()->isCollectionLockedForMode(ns().toString(), MODE_X));
- massert(17445, "index build in progress", _indexCatalog.numIndexesInProgress(txn) == 0);
+ BackgroundOperation::assertNoBgOpInProgForNs(ns());
+ invariant(_indexCatalog.numIndexesInProgress(txn) == 0);
// 1) store index specs
vector<BSONObj> indexSpecs;
@@ -517,6 +519,8 @@ Status Collection::truncate(OperationContext* txn) {
void Collection::temp_cappedTruncateAfter(OperationContext* txn, RecordId end, bool inclusive) {
dassert(txn->lockState()->isCollectionLockedForMode(ns().toString(), MODE_IX));
invariant(isCapped());
+ BackgroundOperation::assertNoBgOpInProgForNs(ns());
+ invariant(_indexCatalog.numIndexesInProgress(txn) == 0);
_cursorManager.invalidateAll(false);
_recordStore->temp_cappedTruncateAfter(txn, end, inclusive);
diff --git a/src/mongo/db/catalog/cursor_manager.cpp b/src/mongo/db/catalog/cursor_manager.cpp
index 6507b8fd1a5..d192d52ba25 100644
--- a/src/mongo/db/catalog/cursor_manager.cpp
+++ b/src/mongo/db/catalog/cursor_manager.cpp
@@ -34,6 +34,7 @@
#include "mongo/base/init.h"
#include "mongo/db/audit.h"
#include "mongo/db/auth/authorization_session.h"
+#include "mongo/db/background.h"
#include "mongo/db/catalog/collection.h"
#include "mongo/db/catalog/database.h"
#include "mongo/db/catalog/database_holder.h"
@@ -313,6 +314,7 @@ CursorManager::~CursorManager() {
void CursorManager::invalidateAll(bool collectionGoingAway) {
SimpleMutex::scoped_lock lk(_mutex);
+ fassert(28819, !BackgroundOperation::inProgForNs(_nss));
for (ExecSet::iterator it = _nonCachedExecutors.begin(); it != _nonCachedExecutors.end();
++it) {
diff --git a/src/mongo/db/catalog/database.cpp b/src/mongo/db/catalog/database.cpp
index 0d6f0e9aef2..652717dddce 100644
--- a/src/mongo/db/catalog/database.cpp
+++ b/src/mongo/db/catalog/database.cpp
@@ -432,6 +432,8 @@ Status Database::renameCollection(OperationContext* txn,
bool stayTemp) {
audit::logRenameCollection(currentClient.get(), fromNS, toNS);
invariant(txn->lockState()->isDbLockedForMode(name(), MODE_X));
+ BackgroundOperation::assertNoBgOpInProgForNs(fromNS);
+ BackgroundOperation::assertNoBgOpInProgForNs(toNS);
{ // remove anything cached
Collection* coll = getCollection(fromNS);
diff --git a/src/mongo/db/catalog/index_catalog.cpp b/src/mongo/db/catalog/index_catalog.cpp
index ceef866f8de..c7752655ee5 100644
--- a/src/mongo/db/catalog/index_catalog.cpp
+++ b/src/mongo/db/catalog/index_catalog.cpp
@@ -744,15 +744,21 @@ Status IndexCatalog::_dropIndex(OperationContext* txn, IndexCatalogEntry* entry)
if (!status.isOK())
return status;
- // there may be pointers pointing at keys in the btree(s). kill them.
- // TODO: can this can only clear cursors on this index?
- _collection->getCursorManager()->invalidateAll(false);
-
// wipe out stats
_collection->infoCache()->reset(txn);
- string indexNamespace = entry->descriptor()->indexNamespace();
+ // Pulling indexName/indexNamespace out as they are needed post descriptor release.
string indexName = entry->descriptor()->indexName();
+ string indexNamespace = entry->descriptor()->indexNamespace();
+
+ // If any cursors could be using this index, invalidate them. Note that we do not use indexes
+ // until they are ready, so we do not need to invalidate anything if the index fails while it is
+ // being built.
+ // TODO only kill cursors that are actually using the index rather than everything on this
+ // collection.
+ if (entry->isReady(txn)) {
+ _collection->getCursorManager()->invalidateAll(false);
+ }
// --------- START REAL WORK ----------
@@ -966,6 +972,7 @@ const IndexCatalogEntry* IndexCatalog::getEntry(const IndexDescriptor* desc) con
const IndexDescriptor* IndexCatalog::refreshEntry(OperationContext* txn,
const IndexDescriptor* oldDesc) {
invariant(txn->lockState()->isCollectionLockedForMode(_collection->ns().ns(), MODE_X));
+ invariant(!BackgroundOperation::inProgForNs(_collection->ns()));
const std::string indexName = oldDesc->indexName();
invariant(_collection->getCatalogEntry()->isIndexReady(txn, indexName));
diff --git a/src/mongo/db/catalog/index_create.cpp b/src/mongo/db/catalog/index_create.cpp
index dcf70e5c3a0..3209b6cfbf5 100644
--- a/src/mongo/db/catalog/index_create.cpp
+++ b/src/mongo/db/catalog/index_create.cpp
@@ -45,6 +45,7 @@
#include "mongo/db/clientcursor.h"
#include "mongo/db/concurrency/write_conflict_exception.h"
#include "mongo/db/curop.h"
+#include "mongo/db/exec/working_set_common.h"
#include "mongo/db/query/internal_plans.h"
#include "mongo/db/repl/oplog.h"
#include "mongo/db/repl/replication_coordinator_global.h"
@@ -285,15 +286,10 @@ Status MultiIndexBlock::insertAllDocumentsInCollection(std::set<RecordId>* dupsO
}
}
- if (state != PlanExecutor::IS_EOF) {
- // If the plan executor was killed, this means the DB/collection was dropped and so it
- // is not safe to cleanup the in-progress indexes.
- if (state == PlanExecutor::DEAD) {
- abortWithoutCleanup();
- }
-
- uasserted(28550, "Unable to complete index build as the collection is no longer readable");
- }
+ uassert(28550,
+ "Unable to complete index build due to collection scan failure: " +
+ WorkingSetCommon::toStatusString(objToIndex.value()),
+ state == PlanExecutor::IS_EOF);
progress->finished();
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index d59ddbfc132..e83a287631b 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -1079,6 +1079,10 @@ public:
Database* const db = autoDb.getDb();
Collection* coll = db ? db->getCollection(ns) : NULL;
+ // This can kill all cursors so don't allow running it while a background operation is in
+ // progress.
+ BackgroundOperation::assertNoBgOpInProgForNs(ns);
+
// If db/collection does not exist, short circuit and return.
if (!db || !coll) {
errmsg = "ns does not exist";
--
GitLab