SERVER-14860 Re-add support for interruptable background index builds on secondaries.

7 files changed, 125 insertions, 40 deletions

diff --git a/src/mongo/db/catalog/index_catalog.cpp b/src/mongo/db/catalog/index_catalog.cpp
index 591bf9cc2b6..176c56925b4 100644
--- a/src/mongo/db/catalog/index_catalog.cpp
+++ b/src/mongo/db/catalog/index_catalog.cpp
@@ -273,6 +273,16 @@ namespace mongo {
         return StatusWith<BSONObj>( fixed );
     }
 
+    void IndexCatalog::registerIndexBuild(IndexDescriptor* descriptor, unsigned int opNum) {
+        _inProgressIndexes[descriptor] = opNum;
+    }
+
+    void IndexCatalog::unregisterIndexBuild(IndexDescriptor* descriptor) {
+        InProgressIndexesMap::iterator it = _inProgressIndexes.find(descriptor);
+        invariant(it != _inProgressIndexes.end());
+        _inProgressIndexes.erase(it);
+    }
+
 namespace {
     class IndexCleanupOnRollback : public RecoveryUnit::Change {
     public:
@@ -1143,10 +1153,38 @@ namespace {
         return b.obj();
     }
 
-    std::vector<BSONObj> IndexCatalog::killMatchingIndexBuilds(
-            const IndexCatalog::IndexKillCriteria& criteria) {
-        // This is just a no-op stub. When SERVER-14860 is resolved, this will either be filled out
-        // or removed entirely.
-        return std::vector<BSONObj>();
+    std::vector<BSONObj>
+    IndexCatalog::killMatchingIndexBuilds(const IndexCatalog::IndexKillCriteria& criteria) {
+        std::vector<BSONObj> indexes;
+        for (InProgressIndexesMap::iterator it = _inProgressIndexes.begin();
+             it != _inProgressIndexes.end();
+             it++) {
+            // check criteria
+            IndexDescriptor* desc = it->first;
+            unsigned int opNum = it->second;
+            if (!criteria.ns.empty() && (desc->parentNS() != criteria.ns)) {
+                continue;
+            }
+            if (!criteria.name.empty() && (desc->indexName() != criteria.name)) {
+                continue;
+            }
+            if (!criteria.key.isEmpty() && (desc->keyPattern() != criteria.key)) {
+                continue;
+            }
+            indexes.push_back(desc->keyPattern().getOwned());
+            log() << "halting index build: " << desc->keyPattern();
+            // Note that we can only be here if the background index build in question is
+            // yielding. The bg index code is set up specially to check for interrupt
+            // immediately after it recovers from yield, such that no further work is done
+            // on the index build. Thus this thread does not have to synchronize with the
+            // bg index operation; we can just assume that it is safe to proceed.
+            getGlobalEnvironment()->killOperation(opNum);
+        }
+
+        if (indexes.size() > 0) {
+            log() << "halted " << indexes.size() << " index build(s)" << endl;
+        }
+
+        return indexes;
     }
 }
diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h
index 00c8d883ab5..3e0ef79306a 100644
--- a/src/mongo/db/catalog/index_catalog.h
+++ b/src/mongo/db/catalog/index_catalog.h
@@ -186,8 +186,27 @@ namespace mongo {
         };
 
         /**
-         * Given some criteria, will search through all in-progress index builds
-         * and will kill ones that match. (namespace, index name, and/or index key spec)
+         * Registers an index build in an internal tracking map, for use with
+         * killMatchingIndexBuilds().  The opNum and descriptor provided must remain active
+         * for as long as the entry exists in the map.  The opNum provided must correspond to
+         * an operation building only one index, in the background.
+         * This function is intended for replication to use for tracking and managing background
+         * index builds.  It is expected that the caller has already taken steps to serialize
+         * calls to this function.
+         */
+        void registerIndexBuild(IndexDescriptor* descriptor, unsigned int opNum);
+
+        /**
+         * Removes an index build from the map, upon completion or termination of the index build.
+         * This function is intended for replication to use for tracking and managing background
+         * index builds.  It is expected that the caller has already taken steps to serialize
+         * calls to this function.
+         */
+        void unregisterIndexBuild(IndexDescriptor* descriptor);
+
+        /**
+         * Given some criteria, searches through all in-progress index builds
+         * and kills ones that match. (namespace, index name, and/or index key spec)
          * Returns the list of index specs that were killed, for use in restarting them later.
          */
         std::vector<BSONObj> killMatchingIndexBuilds(const IndexKillCriteria& criteria);
@@ -274,6 +293,10 @@ namespace mongo {
         static BSONObj fixIndexKey( const BSONObj& key );
 
     private:
+        typedef unordered_map<IndexDescriptor*, unsigned int> InProgressIndexesMap;
+
+        static const BSONObj _idObj; // { _id : 1 }
+
         bool _shouldOverridePlugin( OperationContext* txn, const BSONObj& keyPattern ) const;
 
         /**
@@ -327,13 +350,14 @@ namespace mongo {
 
         IndexCatalogEntryContainer _entries;
 
-        // These are the index specs of indexes that were "leftover"
-        // "Leftover" means they were unfinished when a mongod shut down
-        // Certain operations are prohibted until someone fixes
-        // get by calling getAndClearUnfinishedIndexes
+        // These are the index specs of indexes that were "leftover".
+        // "Leftover" means they were unfinished when a mongod shut down.
+        // Certain operations are prohibited until someone fixes.
+        // Retrieve by calling getAndClearUnfinishedIndexes().
         std::vector<BSONObj> _unfinishedIndexes;
 
-        static const BSONObj _idObj; // { _id : 1 }
+        // Track in-progress index builds, in order to find and stop them when necessary.
+        InProgressIndexesMap _inProgressIndexes;
     };
 
 }
diff --git a/src/mongo/db/catalog/index_create.cpp b/src/mongo/db/catalog/index_create.cpp
index 2f29517d46e..8442972daa6 100644
--- a/src/mongo/db/catalog/index_create.cpp
+++ b/src/mongo/db/catalog/index_create.cpp
@@ -157,7 +157,6 @@ namespace mongo {
             if (!_buildInBackground) {
                 // Bulk build process requires foreground building as it assumes nothing is changing
                 // under it.
-                // TODO SERVER-14860 make background not just be a slower foreground.
                 index.bulk.reset(index.real->initiateBulk(_txn));
             }
 
@@ -190,6 +189,20 @@ namespace mongo {
         return Status::OK();
     }
 
+    IndexDescriptor* MultiIndexBlock::registerIndexBuild() {
+        // Register background index build so that it can be found and killed when necessary
+        invariant(_collection);
+        invariant(_indexes.size() == 1);
+        invariant(_buildInBackground);
+        IndexDescriptor* descriptor = _indexes[0].block->getEntry()->descriptor();
+        _collection->getIndexCatalog()->registerIndexBuild(descriptor, _txn->getCurOp()->opNum());
+        return descriptor;
+    }
+
+    void MultiIndexBlock::unregisterIndexBuild(IndexDescriptor* descriptor) {
+        _collection->getIndexCatalog()->unregisterIndexBuild(descriptor);
+    }
+
     Status MultiIndexBlock::insertAllDocumentsInCollection(std::set<DiskLoc>* dupsOut) {
         const char* curopMessage = _buildInBackground ? "Index Build (background)" : "Index Build";
         ProgressMeter* progress = _txn->setMessage(curopMessage,
@@ -204,6 +217,7 @@ namespace mongo {
                                                                       _collection->ns().ns(),
                                                                       _collection));
         if (_buildInBackground) {
+            invariant(_allowInterruption);
             exec->setYieldPolicy(PlanExecutor::YIELD_AUTO);
         }
 
@@ -212,6 +226,9 @@ namespace mongo {
         PlanExecutor::ExecState state;
         while (PlanExecutor::ADVANCED == (state = exec->getNext(&objToIndex, &loc))) {
             {
+                if (_allowInterruption)
+                    _txn->checkForInterrupt();
+
                 bool shouldCommitWUnit = true;
                 WriteUnitOfWork wunit(_txn);
                 Status ret = insert(objToIndex, loc);
@@ -234,9 +251,6 @@ namespace mongo {
             n++;
             progress->hit();
 
-            if (_allowInterruption)
-                _txn->checkForInterrupt();
-
             progress->setTotalWhileRunning( _collection->numRecords(_txn) );
         }
 
diff --git a/src/mongo/db/catalog/index_create.h b/src/mongo/db/catalog/index_create.h
index bd252d885a9..830eb8bd4e0 100644
--- a/src/mongo/db/catalog/index_create.h
+++ b/src/mongo/db/catalog/index_create.h
@@ -107,6 +107,22 @@ namespace mongo {
         }
 
         /**
+         * Manages in-progress background index builds.
+         * Call registerIndexBuild() after calling init() to record this build in the catalog's 
+         * in-progress map.
+         * The build must be a background build and it must be a single index build (the size of
+         * _indexes must be 1).
+         * registerIndexBuild() returns the descriptor for the index build. You must subsequently
+         * call unregisterIndexBuild() with that same descriptor before this MultiIndexBlock goes
+         * out of scope.
+         * These functions are only intended to be used by the replication system. No internal
+         * concurrency control is performed; it is expected that the code has already taken steps
+         * to ensure calls to these functions are serialized, for a particular IndexCatalog.
+         */
+        IndexDescriptor* registerIndexBuild();
+        void unregisterIndexBuild(IndexDescriptor* descriptor);
+
+        /**
          * Inserts all documents in the Collection into the indexes and logs with timing info.
          *
          * This is a simplified replacement for insert and doneInserting. Do not call this if you
diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index 915b468891b..64d13ca974d 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -76,21 +76,21 @@ namespace mongo {
 
         Database* db = dbHolder().get(&txn, ns.db().toString());
 
-        Status status = build(&txn, db, true);
+        Status status = _build(&txn, db, true);
         if ( !status.isOK() ) {
-            log() << "IndexBuilder could not build index: " << status.toString();
+            error() << "IndexBuilder could not build index: " << status.toString();
         }
 
         txn.getClient()->shutdown();
     }
 
     Status IndexBuilder::buildInForeground(OperationContext* txn, Database* db) const {
-        return build(txn, db, false);
+        return _build(txn, db, false);
     }
 
-    Status IndexBuilder::build(OperationContext* txn,
-                               Database* db,
-                               bool allowBackgroundBuilding) const {
+    Status IndexBuilder::_build(OperationContext* txn,
+                                Database* db,
+                                bool allowBackgroundBuilding) const {
         const string ns = _index["ns"].String();
 
         Collection* c = db->getCollection( txn, ns );
@@ -104,18 +104,22 @@ namespace mongo {
         // Show which index we're building in the curop display.
         txn->getCurOp()->setQuery(_index);
 
-
         MultiIndexBlock indexer(txn, c);
         indexer.allowInterruption();
         if (allowBackgroundBuilding)
             indexer.allowBackgroundBuilding();
 
         Status status = Status::OK();
+        IndexDescriptor* descriptor(NULL);
         try {
             status = indexer.init(_index);
             if ( status.code() == ErrorCodes::IndexAlreadyExists )
                 return Status::OK();
 
+            if (allowBackgroundBuilding) {
+                descriptor = indexer.registerIndexBuild();
+            }
+
             if (status.isOK())
                 status = indexer.insertAllDocumentsInCollection();
 
@@ -133,7 +137,10 @@ namespace mongo {
             // leave it as-if kill -9 happened. This will be handled on restart.
             indexer.abortWithoutCleanup();
         }
-        
+
+        if (allowBackgroundBuilding) {
+            indexer.unregisterIndexBuild(descriptor);
+        }
         return status;
     }
 
diff --git a/src/mongo/db/index_builder.h b/src/mongo/db/index_builder.h
index 29364757526..8d754dde5fc 100644
--- a/src/mongo/db/index_builder.h
+++ b/src/mongo/db/index_builder.h
@@ -76,7 +76,7 @@ namespace mongo {
         static void restoreIndexes(const std::vector<BSONObj>& indexes);
 
     private:
-        Status build(OperationContext* txn, Database* db, bool allowBackgroundBuilding) const;
+        Status _build(OperationContext* txn, Database* db, bool allowBackgroundBuilding) const;
 
         const BSONObj _index;
         std::string _name; // name of this builder, not related to the index
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 0e95edf71ea..0f28482ff74 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -585,21 +585,7 @@ namespace repl {
                 else {
                     IndexBuilder builder(o);
                     Status status = builder.buildInForeground(txn, db);
-                    if ( status.isOK() ) {
-                        // yay
-                    }
-                    else if ( status.code() == ErrorCodes::IndexOptionsConflict ||
-                              status.code() == ErrorCodes::IndexKeySpecsConflict ) {
-                        // SERVER-13206, SERVER-13496
-                        // 2.4 (and earlier) will add an ensureIndex to an oplog if its ok or not
-                        // so in 2.6+ where we do stricter validation, it will fail
-                        // but we shouldn't care as the primary is responsible
-                        warning() << "index creation attempted on secondary that conflicts, "
-                                  << "skipping: " << status;
-                    }
-                    else {
-                        uassertStatusOK( status );
-                    }
+                    uassertStatusOK(status);
                 }
             }
             else {