Move chunk obj counting into mongod.

Was segfaulting because newChunk->_master could become dangling

3 files changed, 46 insertions, 65 deletions

diff --git a/s/chunk.cpp b/s/chunk.cpp
index db686429bad..a2b53cff91f 100644
--- a/s/chunk.cpp
+++ b/s/chunk.cpp
@@ -337,7 +337,37 @@ namespace mongo {
 #endif
                   << endl;
 
-            moveIfShould( shared_from_this() , newShard );
+            BSONElement shouldMigrate = res["shouldMigrate"]; // not in mongod < 1.9.1 but that is ok
+            if (!shouldMigrate.eoo()){
+                BSONObj range = shouldMigrate.embeddedObject();
+                Shard newLocation = Shard::pick( getShard() );
+                if ( getShard() == newLocation ) {
+                    // if this is the best shard, then we shouldn't do anything (Shard::pick already logged our shard).
+                    log(1) << "recently split chunk: " << range << " already in the best shard: " << getShard() << endl;
+                    return true; // we did split even if we didn't migrate
+                }
+
+                BSONObj min = range["min"].embeddedObject();
+                BSONObj max = range["max"].embeddedObject();
+
+                ChunkManagerPtr cm = grid.getDBConfig(getns())->getChunkManager(getns(), false/*reloaded by split*/);
+                ChunkPtr toMove = cm->findChunk(min);
+
+                if ( ! (toMove->getMin() == min && toMove->getMax() == max) ){
+                    log(1) << "recently split chunk: " << range << " modified before we could migrate " << toMove << endl;
+                    return true;
+                }
+
+                log() << "moving chunk (auto): " << toMove << " to: " << newLocation.toString() << endl;
+
+                BSONObj res;
+                massert( 10412 ,
+                         str::stream() << "moveAndCommit failed: " << res ,
+                         toMove->moveAndCommit( newLocation , MaxChunkSize , res ) );
+                
+                // update our config
+                grid.getDBConfig( toMove->getns() )->getChunkManager( toMove->getns() , true );
+            }
 
             return true;
 
@@ -349,41 +379,6 @@ namespace mongo {
         }
     }
 
-    bool Chunk::moveIfShould( ChunkPtr oldChunk , ChunkPtr newChunk ) {
-        ChunkPtr toMove;
-
-        if ( newChunk->countObjects(2) <= 1 ) {
-            toMove = newChunk;
-        }
-        else if ( oldChunk->countObjects(2) <= 1 ) {
-            toMove = oldChunk;
-        }
-        else {
-            // moving middle shards is handled by balancer
-            return false;
-        }
-
-        assert( toMove );
-
-        Shard newLocation = Shard::pick( toMove->getShard() );
-        if ( toMove->getShard() == newLocation ) {
-            // if this is the best shard, then we shouldn't do anything (Shard::pick already logged our shard).
-            log(1) << "recently split chunk: " << toMove->toString() << "already in the best shard" << endl;
-            return false;
-        }
-
-        log() << "moving chunk (auto): " << toMove->toString() << " to: " << newLocation.toString() << " #objects: " << toMove->countObjects() << endl;
-
-        BSONObj res;
-        massert( 10412 ,
-                 str::stream() << "moveAndCommit failed: " << res ,
-                 toMove->moveAndCommit( newLocation , MaxChunkSize , res ) );
-        
-        // update our config
-        grid.getDBConfig( toMove->getns() )->getChunkManager( toMove->getns() , true );
-        return true;
-    }
-
     long Chunk::getPhysicalSize() const {
         ScopedDbConnection conn( getShard().getConnString() );
 
@@ -401,23 +396,6 @@ namespace mongo {
         return (long)result["size"].number();
     }
 
-    int Chunk::countObjects(int maxCount) const {
-        static const BSONObj fields = BSON("_id" << 1 );
-
-        ShardConnection conn( getShard() , _manager->getns() );
-
-        // not using regular count as this is more flexible and supports $min/$max
-        Query q = Query().minKey(_min).maxKey(_max);
-        int n;
-        {
-            auto_ptr<DBClientCursor> c = conn->query(_manager->getns(), q, maxCount, 0, &fields);
-            assert( c.get() );
-            n = c->itcount();
-        }
-        conn.done();
-        return n;
-    }
-
     void Chunk::appendShortVersion( const char * name , BSONObjBuilder& b ) {
         BSONObjBuilder bb( b.subobjStart( name ) );
         bb.append( "min" , _min );
diff --git a/s/chunk.h b/s/chunk.h
index 293d9a32431..05a21a640b0 100644
--- a/s/chunk.h
+++ b/s/chunk.h
@@ -141,13 +141,6 @@ namespace mongo {
         //
 
         /**
-         * moves either this shard or newShard if it makes sense too
-         *
-         * @return whether or not a shard was moved
-         */
-        static bool moveIfShould( ChunkPtr oldChunk , ChunkPtr newChunk );
-
-        /**
          * Issues a migrate request for this chunk
          *
          * @param to shard to move this chunk to
@@ -164,11 +157,6 @@ namespace mongo {
         long getPhysicalSize() const;
 
         //
-        // chunk size support
-
-        int countObjects(int maxcount=0) const;
-
-        //
         // public constants
         //
 
diff --git a/s/d_split.cpp b/s/d_split.cpp
index b28878f2aac..e19030187ff 100644
--- a/s/d_split.cpp
+++ b/s/d_split.cpp
@@ -802,6 +802,21 @@ namespace mongo {
                 }
             }
 
+            if (newChunks.size() == 2){
+                // If one of the chunks has only one object in it we should move it
+                static const BSONObj fields = BSON("_id" << 1 );
+                DBDirectClient conn;
+                for (int i=1; i >= 0 ; i--){ // high chunk more likely to have only one obj
+                    ChunkInfo chunk = newChunks[i];
+                    Query q = Query().minKey(chunk.min).maxKey(chunk.max);
+                    scoped_ptr<DBClientCursor> c (conn.query(ns, q, /*limit*/-2, 0, &fields));
+                    if (c && c->itcount() == 1) {
+                        result.append("shouldMigrate", BSON("min" << chunk.min << "max" << chunk.max));
+                        break;
+                    }
+                }
+            }
+
             return true;
         }
     } cmdSplitChunk;