Revert "SERVER-7376 make all migration cleanup async unless explicitly specified otherwise"

This reverts commit dfb8e2604409e2879ea975c4c49ba6ffde90d795.

7 files changed, 33 insertions, 95 deletions

diff --git a/src/mongo/s/balance.cpp b/src/mongo/s/balance.cpp
index e12ecbf58a4..248b3e4d8d5 100644
--- a/src/mongo/s/balance.cpp
+++ b/src/mongo/s/balance.cpp
@@ -40,10 +40,7 @@ namespace mongo {
     Balancer::~Balancer() {
     }
 
-    int Balancer::_moveChunks( const vector<CandidateChunkPtr>* candidateChunks ,
-                               bool secondaryThrottle,
-                               bool waitForDelete )
-    {
+    int Balancer::_moveChunks( const vector<CandidateChunkPtr>* candidateChunks , bool secondaryThrottle ) {
         int movedCount = 0;
 
         for ( vector<CandidateChunkPtr>::const_iterator it = candidateChunks->begin(); it != candidateChunks->end(); ++it ) {
@@ -69,12 +66,7 @@ namespace mongo {
             }
 
             BSONObj res;
-            if ( c->moveAndCommit( Shard::make( chunkInfo.to ) ,
-                                   Chunk::MaxChunkSize ,
-                                   secondaryThrottle ,
-                                   waitForDelete,
-                                   res ) )
-            {
+            if ( c->moveAndCommit( Shard::make( chunkInfo.to ) , Chunk::MaxChunkSize , secondaryThrottle , res ) ) {
                 movedCount++;
                 continue;
             }
@@ -338,7 +330,7 @@ namespace mongo {
                 }
 
                 sleepTime = balancerConfig["_nosleep"].trueValue() ? 30 : 6;
-
+                
                 uassert( 13258 , "oids broken after resetting!" , _checkOIDs() );
 
                 {
@@ -357,14 +349,6 @@ namespace mongo {
                     
                     LOG(1) << "*** start balancing round" << endl;
 
-                    if (balancerConfig["_waitForDelete"].trueValue()) {
-                        LOG(1) << "balancer chunk moves will wait for cleanup" << endl;
-                    }
-
-                    if (balancerConfig["_secondaryThrottle"].trueValue()) {
-                        LOG(1) << "balancer chunk moves will wait for secondaries" << endl;
-                    }
-
                     vector<CandidateChunkPtr> candidateChunks;
                     _doBalanceRound( conn.conn() , &candidateChunks );
                     if ( candidateChunks.size() == 0 ) {
@@ -372,12 +356,9 @@ namespace mongo {
                         _balancedLastTime = 0;
                     }
                     else {
-                        _balancedLastTime =
-                                _moveChunks( &candidateChunks,
-                                             balancerConfig["_secondaryThrottle"].trueValue(),
-                                             balancerConfig["_waitForDelete"].trueValue());
+                        _balancedLastTime = _moveChunks( &candidateChunks, balancerConfig["_secondaryThrottle"].trueValue() );
                     }
-
+                    
                     LOG(1) << "*** end of balancing round" << endl;
                 }
 
diff --git a/src/mongo/s/balance.h b/src/mongo/s/balance.h
index 26b471913e0..98221c5d4db 100644
--- a/src/mongo/s/balance.h
+++ b/src/mongo/s/balance.h
@@ -82,13 +82,9 @@ namespace mongo {
          * Issues chunk migration request, one at a time.
          *
          * @param candidateChunks possible chunks to move
-         * @param secondaryThrottle wait for secondaries to catch up before pushing more deletes
-         * @param waitForDelete wait for deletes to complete after each chunk move
          * @return number of chunks effectively moved
          */
-        int _moveChunks( const vector<CandidateChunkPtr>* candidateChunks,
-                         bool secondaryThrottle,
-                         bool waitForDelete );
+        int _moveChunks( const vector<CandidateChunkPtr>* candidateChunks , bool secondaryThrottle );
 
         /**
          * Marks this balancer as being live on the config server(s).
diff --git a/src/mongo/s/chunk.cpp b/src/mongo/s/chunk.cpp
index eace90f9e7d..773978014dc 100644
--- a/src/mongo/s/chunk.cpp
+++ b/src/mongo/s/chunk.cpp
@@ -301,12 +301,7 @@ namespace mongo {
         return true;
     }
 
-    bool Chunk::moveAndCommit( const Shard& to,
-                               long long chunkSize /* bytes */,
-                               bool secondaryThrottle,
-                               bool waitForDelete,
-                               BSONObj& res ) const
-    {
+    bool Chunk::moveAndCommit( const Shard& to , long long chunkSize /* bytes */, bool secondaryThrottle, BSONObj& res ) const {
         uassert( 10167 ,  "can't move shard to its current location!" , getShard() != to );
 
         log() << "moving chunk ns: " << _manager->getns() << " moving ( " << toString() << ") " << _shard.toString() << " -> " << to.toString() << endl;
@@ -329,8 +324,7 @@ namespace mongo {
                                                          "maxChunkSizeBytes" << chunkSize <<
                                                          "shardId" << genID() <<
                                                          "configdb" << configServer.modelServer() <<
-                                                         "secondaryThrottle" << secondaryThrottle <<
-                                                         "waitForDelete" << waitForDelete
+                                                         "secondaryThrottle" << secondaryThrottle
                                                          ) ,
                                                    res
                                                    );
@@ -431,7 +425,6 @@ namespace mongo {
                          toMove->moveAndCommit( newLocation , 
                                                 MaxChunkSize , 
                                                 false , /* secondaryThrottle - small chunk, no need */
-                                                false , /* waitForDelete - small chunk, no need */
                                                 res ) );
                 
                 // update our config
diff --git a/src/mongo/s/chunk.h b/src/mongo/s/chunk.h
index 11f910ea52c..5fd01c5abe4 100644
--- a/src/mongo/s/chunk.h
+++ b/src/mongo/s/chunk.h
@@ -146,15 +146,10 @@ namespace mongo {
          * @param to shard to move this chunk to
          * @param chunSize maximum number of bytes beyond which the migrate should no go trhough
          * @param secondaryThrottle whether during migrate all writes should block for repl
-         * @param waitForDelete whether chunk move should wait for cleanup or return immediately
          * @param res the object containing details about the migrate execution
          * @return true if move was successful
          */
-        bool moveAndCommit( const Shard& to,
-                            long long chunkSize,
-                            bool secondaryThrottle,
-                            bool waitForDelete,
-                            BSONObj& res ) const;
+        bool moveAndCommit( const Shard& to , long long chunkSize , bool secondaryThrottle, BSONObj& res ) const;
 
         /**
          * @return size of shard in bytes
diff --git a/src/mongo/s/commands_admin.cpp b/src/mongo/s/commands_admin.cpp
index 3ccd3e58f98..c155e25066d 100644
--- a/src/mongo/s/commands_admin.cpp
+++ b/src/mongo/s/commands_admin.cpp
@@ -844,12 +844,7 @@ namespace mongo {
                 }
 
                 BSONObj res;
-                if ( ! c->moveAndCommit( to,
-                                         maxChunkSizeBytes,
-                                         cmdObj["_secondaryThrottle"].trueValue(),
-                                         cmdObj["_waitForDelete"].trueValue(),
-                                         res ) )
-                {
+                if ( ! c->moveAndCommit( to , maxChunkSizeBytes , cmdObj["_secondaryThrottle"].trueValue() , res ) ) {
                     errmsg = "move failed";
                     result.append( "cause" , res );
                     return false;
diff --git a/src/mongo/s/d_migrate.cpp b/src/mongo/s/d_migrate.cpp
index 008cd4d6b12..f5d59566b71 100644
--- a/src/mongo/s/d_migrate.cpp
+++ b/src/mongo/s/d_migrate.cpp
@@ -255,9 +255,7 @@ namespace mongo {
     class MigrateFromStatus {
     public:
 
-        MigrateFromStatus() : _m("MigrateFromStatus") ,
-                              _cleanupTickets(1) /* only one cleanup thread at once */
-        {
+        MigrateFromStatus() : _m("MigrateFromStatus") , _workLock("MigrateFromStatus::workLock") {
             _active = false;
             _inCriticalSection = false;
             _memoryUsed = 0;
@@ -267,9 +265,7 @@ namespace mongo {
                     const BSONObj& min ,
                     const BSONObj& max ,
                     const BSONObj& shardKeyPattern ) {
-
-            // Note: migrations not blocked by queued deletes using _workLock
-
+            scoped_lock ll(_workLock);
             scoped_lock l(_m); // reads and writes _active
 
             verify( ! _active );
@@ -338,7 +334,7 @@ namespace mongo {
 
             case 'd': {
 
-                if (getThreadName().find(cleanUpThreadName) == 0) {
+                if ( getThreadName() == cleanUpThreadName ) {
                     // we don't want to xfer things we're cleaning
                     // as then they'll be deleted on TO
                     // which is bad
@@ -612,12 +608,18 @@ namespace mongo {
         bool isActive() const { return _getActive(); }
         
         void doRemove( OldDataCleanup& cleanup ) {
-
-            log() << "waiting to remove documents for " << cleanup.toString() << endl;
-
-            ScopedTicket ticket(&_cleanupTickets);
-
-            cleanup.doRemove();
+            int it = 0;
+            while ( true ) { 
+                if ( it > 20 && it % 10 == 0 ) log() << "doRemote iteration " << it << " for: " << cleanup << endl;
+                {
+                    scoped_lock ll(_workLock);
+                    if ( ! _active ) {
+                        cleanup.doRemove();
+                        return;
+                    }
+                }
+                sleepmillis( 1000 );
+            }
         }
 
     private:
@@ -644,8 +646,8 @@ namespace mongo {
         list<BSONObj> _deleted; // objects deleted during clone that should be deleted later
         long long _memoryUsed; // bytes in _reload + _deleted
 
-        // this is used to make sure only a certain number of threads are doing cleanup at once.
-        mutable TicketHolder _cleanupTickets;
+        mutable mongo::mutex _workLock; // this is used to make sure only 1 thread is doing serious work
+                                        // for now, this means migrate or removing old chunk data
 
         bool _getActive() const { scoped_lock l(_m); return _active; }
         void _setActive( bool b ) { scoped_lock l(_m); _active = b; }
@@ -665,10 +667,7 @@ namespace mongo {
     };
 
     void _cleanupOldData( OldDataCleanup cleanup ) {
-
-        Client::initThread((string(cleanUpThreadName) + string("-") +
-                                                        OID::gen().toString()).c_str());
-
+        Client::initThread( cleanUpThreadName );
         if (!noauth) {
             cc().getAuthenticationInfo()->authorize("local", internalSecurity.user);
         }
@@ -809,12 +808,6 @@ namespace mongo {
                 warning() << "secondaryThrottle selected but no replication" << endl;
             }
 
-            // Do inline deletion
-            bool waitForDelete = cmdObj["waitForDelete"].trueValue();
-            if (waitForDelete) {
-                log() << "moveChunk waiting for full cleanup after move" << endl;
-            }
-
             BSONObj min  = cmdObj["min"].Obj();
             BSONObj max  = cmdObj["max"].Obj();
             BSONElement shardId = cmdObj["shardId"];
@@ -1341,17 +1334,17 @@ namespace mongo {
                 c.max = max.getOwned();
                 c.shardKeyPattern = shardKeyPattern.getOwned();
                 ClientCursor::find( ns , c.initial );
-
-                if (!waitForDelete) {
-                    // 7.
-                    log() << "forking for cleanup of chunk data" << migrateLog;
+                if ( c.initial.size() ) {
+                    log() << "forking for cleaning up chunk data" << migrateLog;
                     boost::thread t( boost::bind( &cleanupOldData , c ) );
                 }
                 else {
+                    log() << "doing delete inline" << migrateLog;
                     // 7.
-                    log() << "doing delete inline for cleanup of chunk data" << migrateLog;
                     c.doRemove();
                 }
+
+
             }
             timing.done(6);
 
diff --git a/src/mongo/util/concurrency/ticketholder.h b/src/mongo/util/concurrency/ticketholder.h
index c32e283e130..036d33fb9ef 100644
--- a/src/mongo/util/concurrency/ticketholder.h
+++ b/src/mongo/util/concurrency/ticketholder.h
@@ -95,21 +95,6 @@ namespace mongo {
         boost::condition_variable_any _newTicket;
     };
 
-    class ScopedTicket {
-    public:
-
-        ScopedTicket(TicketHolder* holder) : _holder(holder) {
-            _holder->waitForTicket();
-        }
-
-        ~ScopedTicket() {
-            _holder->release();
-        }
-
-    private:
-        TicketHolder* _holder;
-    };
-
     class TicketHolderReleaser {
     public:
         TicketHolderReleaser( TicketHolder * holder ) {