renameCollection dangling iterator

32 views
Skip to first unread message

Michael Cahill

unread,
Sep 2, 2014, 7:27:46 AM9/2/14
to mongo...@googlegroups.com
WiredTiger checks for open cursors during drop and rename operations.  This causes problems in the rename7.js test, which renames a collection between databases.  That is implemented as a loop that copies the records followed by a drop.  The problem for WiredTiger is that the iterator used for the copy isn't closed before drop.

The patch below fixes the problem.  Is this fix okay?

Would this kind of thing be better as a Github pull request?

Thanks,
Michael.


diff --git a/src/mongo/db/commands/rename_collection.cpp b/src/mongo/db/commands/rename_collection.cpp
--- a/src/mongo/db/commands/rename_collection.cpp
+++ b/src/mongo/db/commands/rename_collection.cpp
@@ -270,19 +270,21 @@ namespace mongo {
                 indexer.init(indexesToCopy);
             }

-            // Copy over all the data from source collection to target collection.
-            boost::scoped_ptr<RecordIterator> sourceIt(sourceColl->getIterator(txn));
-            while (!sourceIt->isEOF()) {
-                txn->checkForInterrupt(false);
+            {
+                // Copy over all the data from source collection to target collection.
+                boost::scoped_ptr<RecordIterator> sourceIt(sourceColl->getIterator(txn));
+                while (!sourceIt->isEOF()) {
+                    txn->checkForInterrupt(false);

-                const BSONObj obj = sourceColl->docFor(txn, sourceIt->getNext());
+                    const BSONObj obj = sourceColl->docFor(txn, sourceIt->getNext());

-                WriteUnitOfWork wunit(txn);
-                // No logOp necessary because the entire renameCollection command is one logOp.
-                Status status = targetColl->insertDocument(txn, obj, &indexer, true).getStatus();
-                if (!status.isOK())
-                    return appendCommandStatus(result, status);
-                wunit.commit();
+                    WriteUnitOfWork wunit(txn);
+                    // No logOp necessary because the entire renameCollection command is one logOp.
+                    Status status = targetColl->insertDocument(txn, obj, &indexer, true).getStatus();
+                    if (!status.isOK())
+                        return appendCommandStatus(result, status);
+                    wunit.commit();
+                }
             }

             Status status = indexer.doneInserting();


Eliot Horowitz

unread,
Sep 2, 2014, 9:04:56 AM9/2/14
to mongo...@googlegroups.com
The theory makes total sense.
Something seems off in formatting, but its probably just a google groups things.
Pull request definitely the best thing to do to get merged fast.


--
You received this message because you are subscribed to the Google Groups "mongodb-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mongodb-dev...@googlegroups.com.
To post to this group, send email to mongo...@googlegroups.com.
Visit this group at http://groups.google.com/group/mongodb-dev.
For more options, visit https://groups.google.com/d/optout.

Michael Cahill

unread,
Sep 2, 2014, 9:22:44 PM9/2/14
to mongo...@googlegroups.com
On Tuesday, September 2, 2014 11:04:56 PM UTC+10, Eliot wrote:
The theory makes total sense.
Something seems off in formatting, but its probably just a google groups things.
Pull request definitely the best thing to do to get merged fast.

Thanks, Eliot.  I've opened SERVER-15121 and Fix dangling iterator in renameCollection across databases with this change.

Michael.
Reply all
Reply to author
Forward
0 new messages