SERVER-16274 synchronize start of bg index builds on secondaries

8 files changed, 75 insertions, 19 deletions

diff --git a/src/mongo/db/commands/compact.cpp b/src/mongo/db/commands/compact.cpp
index 78af3ed4150..c5e4ea9a53a 100644
--- a/src/mongo/db/commands/compact.cpp
+++ b/src/mongo/db/commands/compact.cpp
@@ -176,7 +176,7 @@ namespace mongo {
 
             log() << "compact " << ns << " end";
 
-            IndexBuilder::restoreIndexes(indexesInProg);
+            IndexBuilder::restoreIndexes(txn, indexesInProg);
 
             return true;
         }
diff --git a/src/mongo/db/commands/drop_indexes.cpp b/src/mongo/db/commands/drop_indexes.cpp
index ee4683dc0d5..62e62c5c566 100644
--- a/src/mongo/db/commands/drop_indexes.cpp
+++ b/src/mongo/db/commands/drop_indexes.cpp
@@ -314,7 +314,7 @@ namespace mongo {
             result.append( "nIndexes", (int)all.size() );
             result.append( "indexes", all );
 
-            IndexBuilder::restoreIndexes(indexesInProg);
+            IndexBuilder::restoreIndexes(txn, indexesInProg);
             return true;
         }
     } cmdReIndex;
diff --git a/src/mongo/db/commands/rename_collection.cpp b/src/mongo/db/commands/rename_collection.cpp
index 9674067afa5..31f26bbb278 100644
--- a/src/mongo/db/commands/rename_collection.cpp
+++ b/src/mongo/db/commands/rename_collection.cpp
@@ -93,10 +93,6 @@ namespace mongo {
             return indexes;
         }
 
-        virtual void restoreIndexBuildsOnSource(std::vector<BSONObj> indexesInProg, std::string source) {
-            IndexBuilder::restoreIndexes( indexesInProg );
-        }
-
         static void dropCollection(OperationContext* txn, Database* db, StringData collName) {
             WriteUnitOfWork wunit(txn);
             if (db->dropCollection(txn, collName).isOK()) {
@@ -203,7 +199,9 @@ namespace mongo {
 
             const std::vector<BSONObj> indexesInProg = stopIndexBuilds(txn, sourceDB, cmdObj);
             // Dismissed on success
-            ScopeGuard indexBuildRestorer = MakeGuard(IndexBuilder::restoreIndexes, indexesInProg);
+            ScopeGuard indexBuildRestorer = MakeGuard(IndexBuilder::restoreIndexes,
+                                                      txn,
+                                                      indexesInProg);
 
             Database* const targetDB = dbHolder().openDb(txn, nsToDatabase(target));
 
diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp
index 44c41a8df93..0e9336858c4 100644
--- a/src/mongo/db/commands/test_commands.cpp
+++ b/src/mongo/db/commands/test_commands.cpp
@@ -221,7 +221,7 @@ namespace mongo {
                 return appendCommandStatus(result, status);
             }
 
-            IndexBuilder::restoreIndexes(indexes);
+            IndexBuilder::restoreIndexes(txn, indexes);
 
             if (!fromRepl) {
                 repl::logOp(txn, "c", (dbname + ".$cmd").c_str(), cmdObj);
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index 5ef335b7138..0453a3ca4c8 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -291,7 +291,7 @@ namespace mongo {
             Status status = repairDatabase(txn, engine, dbname, preserveClonedFilesOnFailure,
                                            backupOriginalFiles );
 
-            IndexBuilder::restoreIndexes(indexesInProg);
+            IndexBuilder::restoreIndexes(txn, indexesInProg);
 
             // Open database before returning
             dbHolder().openDb(txn, dbname);
diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index 8e7d38fd8a3..95413374f4b 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -47,6 +47,22 @@ namespace mongo {
 
     AtomicUInt32 IndexBuilder::_indexBuildCount;
 
+namespace {
+    // Synchronization tools when replication spawns a background index in a new thread.
+    // The bool is 'true' when a new background index has started in a new thread but the
+    // parent thread has not yet synchronized with it.
+    bool _bgIndexStarting(false);
+    boost::mutex _bgIndexStartingMutex;
+    boost::condition_variable _bgIndexStartingCondVar;
+
+    void _setBgIndexStarting() {
+        boost::mutex::scoped_lock lk(_bgIndexStartingMutex);
+        invariant(_bgIndexStarting == false);
+        _bgIndexStarting = true;
+        _bgIndexStartingCondVar.notify_one();
+    }
+} // namespace
+
     IndexBuilder::IndexBuilder(const BSONObj& index) :
         BackgroundJob(true /* self-delete */), _index(index.getOwned()),
         _name(str::stream() << "repl index builder " << _indexBuildCount.addAndFetch(1)) {
@@ -90,6 +106,15 @@ namespace mongo {
         return _build(txn, db, false, NULL);
     }
 
+    void IndexBuilder::waitForBgIndexStarting() {
+        boost::unique_lock<boost::mutex> lk(_bgIndexStartingMutex);
+        while (_bgIndexStarting == false) {
+            _bgIndexStartingCondVar.wait(lk);
+        }
+        // Reset for next time.
+        _bgIndexStarting = false;
+    }
+
     Status IndexBuilder::_build(OperationContext* txn,
                                 Database* db,
                                 bool allowBackgroundBuilding,
@@ -109,6 +134,7 @@ namespace mongo {
 
         MultiIndexBlock indexer(txn, c);
         indexer.allowInterruption();
+
         if (allowBackgroundBuilding)
             indexer.allowBackgroundBuilding();
 
@@ -116,12 +142,18 @@ namespace mongo {
         IndexDescriptor* descriptor(NULL);
         try {
             status = indexer.init(_index);
-            if ( status.code() == ErrorCodes::IndexAlreadyExists )
+            if ( status.code() == ErrorCodes::IndexAlreadyExists ) {
+                if (allowBackgroundBuilding) {
+                    // Must set this in case anyone is waiting for this build.
+                    _setBgIndexStarting();
+                }
                 return Status::OK();
+            }
 
             if (status.isOK()) {
                 if (allowBackgroundBuilding) {
                     descriptor = indexer.registerIndexBuild();
+                    _setBgIndexStarting();
                     invariant(dbLock);
                     dbLock->relockWithMode(MODE_IX);
                 }
@@ -159,20 +191,21 @@ namespace mongo {
         return status;
     }
 
-    std::vector<BSONObj> 
+    std::vector<BSONObj>
     IndexBuilder::killMatchingIndexBuilds(Collection* collection,
                                           const IndexCatalog::IndexKillCriteria& criteria) {
         invariant(collection);
         return collection->getIndexCatalog()->killMatchingIndexBuilds(criteria);
     }
 
-    void IndexBuilder::restoreIndexes(const std::vector<BSONObj>& indexes) {
-        log() << "restarting " << indexes.size() << " index build(s)" << endl;
+    void IndexBuilder::restoreIndexes(OperationContext* txn, const std::vector<BSONObj>& indexes) {
+        log() << "restarting " << indexes.size() << " background index build(s)" << endl;
         for (int i = 0; i < static_cast<int>(indexes.size()); i++) {
             IndexBuilder* indexBuilder = new IndexBuilder(indexes[i]);
             // This looks like a memory leak, but indexBuilder deletes itself when it finishes
             indexBuilder->go();
+            Lock::TempRelease release(txn->lockState());
+            IndexBuilder::waitForBgIndexStarting();
         }
     }
 }
-
diff --git a/src/mongo/db/index_builder.h b/src/mongo/db/index_builder.h
index 2c3c5eaecee..b32c6b5d988 100644
--- a/src/mongo/db/index_builder.h
+++ b/src/mongo/db/index_builder.h
@@ -43,7 +43,22 @@ namespace mongo {
     class OperationContext;
 
     /**
-     * Forks off a thread to build an index.
+     * A helper class for replication to use for building indexes.
+     * In standalone mode, we use the client connection thread for building indexes in the
+     * background. In replication mode, secondaries must spawn a new thread to build background 
+     * indexes, since there are no client connection threads to use for such purpose.  IndexBuilder
+     * is a subclass of BackgroundJob to enable this use.
+     * This class is also used for building indexes in the foreground on secondaries, for
+     * code convenience.  buildInForeground() is directly called by the replication applier to
+     * build an index in the foreground; the properties of BackgroundJob are not used for this use
+     * case.
+     * For background index builds, BackgroundJob::go() is called on the IndexBuilder instance,
+     * which begins a new thread at this class's run() method.  After go() is called in the
+     * parent thread, waitForBgIndexStarting() must be called by the same parent thread,
+     * before any other thread calls go() on any other IndexBuilder instance.  This is
+     * ensured by the replication system, since commands are effectively run single-threaded
+     * by the replication applier, and index builds are treated as commands even though they look
+     * like inserts on system.indexes.
      */
     class IndexBuilder : public BackgroundJob {
     public:
@@ -64,7 +79,7 @@ namespace mongo {
          * index ns, index name, and/or index key spec.
          * Returns a vector of the indexes that were killed.
          */
-        static std::vector<BSONObj> 
+        static std::vector<BSONObj>
             killMatchingIndexBuilds(Collection* collection,
                                     const IndexCatalog::IndexKillCriteria& criteria);
 
@@ -73,11 +88,18 @@ namespace mongo {
          * not match the ns field in the indexes list, the BSONObj's ns field is changed before the
          * index is built (to handle rename).
          */
-        static void restoreIndexes(const std::vector<BSONObj>& indexes);
+        static void restoreIndexes(OperationContext* txn, const std::vector<BSONObj>& indexes);
+
+        /**
+         * Waits for a background index build to register itself.  This function must be called
+         * after starting a background index build via a BackgroundJob and before starting a
+         * subsequent one.
+         */
+        static void waitForBgIndexStarting();
 
     private:
-        Status _build(OperationContext* txn, 
-                      Database* db, 
+        Status _build(OperationContext* txn,
+                      Database* db,
                       bool allowBackgroundBuilding,
                       Lock::DBLock* dbLock) const;
 
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 60e7a216a7e..899e8f32a49 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -606,6 +606,9 @@ namespace repl {
                     IndexBuilder* builder = new IndexBuilder(o);
                     // This spawns a new thread and returns immediately.
                     builder->go();
+                    // Wait for thread to start and register itself
+                    Lock::TempRelease release(txn->lockState());
+                    IndexBuilder::waitForBgIndexStarting();
                 }
                 else {
                     IndexBuilder builder(o);