-
Type:
Bug
-
Resolution: Done
-
Priority:
Major - P3
-
Affects Version/s: None
-
Component/s: Concurrency
-
None
-
ALL
-
None
-
3
-
None
-
None
-
None
-
None
-
None
-
None
Before a document is deleted, all cursors pointing at it are advanced until they do not point at it:
c->recoverFromYield();
DiskLoc tmp1 = c->refLoc();
if ( tmp1 != dl ) {
// This might indicate a failure to call ClientCursor::prepareToYield() but it can
// also happen during correct operation, see SERVER-2009.
problem() << "warning: cursor loc " << tmp1 << " does not match byLoc position " << dl << " !" << endl;
}
else {
c->advance();
}
while (!c->eof() && c->refLoc() == dl) {
/* We don't delete at EOF because we want to return "no more results" rather than "no such cursor".
* The loop is to handle MultiKey indexes where the deleted record is pointed to by multiple adjacent keys.
* In that case we need to advance until we get to the next distinct record or EOF.
* SERVER-4154
* SERVER-5198
* But see SERVER-5725.
*/
c->advance();
}
cc->updateLocation();
Some of the functions above may generate page fault exceptions for certain types of Cursors. For example an inconsistency can arise if:
- A cursor is advanced but an exception is thrown before cc->updateLocation() is called, causing the cursor's _lastLoc to be inconsistent with its actual location.
- A page fault exception is thrown within advance(), leaving the cursor in an inconsistent state.
The impact of these issues is somewhat mitigated by the fact that a non capped BasicCursor tends to perform few data accesses, and that a BtreeCursor probably does not need to be handled in aboutToDelete() anyway (see SERVER-5725). I have not checked all cases with these cursor types, and with the potential for page fault exceptions in this code there are a lot of cases to check. For BtreeCursor there at least seems to be a problematic case in noteLocation() where page fault exceptions are simulated in debug mode and keyAtKeyOfs is updated but locAtKeyOfs is not updated due to a page fault exception. This could cause the cursor to skip matching documents.
I am attaching a test case for one issue where a MultiCusor is left in an inconsistent state due to a page fault exception in advance(). I believe there is also a case where a MultiCusor can be left pointing to a bad document, but it is difficult to write a test for that case.
This test will fail sporadically on debug builds, where a page fault is simulated probabilistically:
diff --git a/src/mongo/dbtests/cursortests.cpp b/src/mongo/dbtests/cursortests.cpp
index 38d1197..738f62b 100644
--- a/src/mongo/dbtests/cursortests.cpp
+++ b/src/mongo/dbtests/cursortests.cpp
@@ -298,7 +298,51 @@ namespace CursorTests {
client.dropCollection( ns() );
}
};
-
+
+ /**
+ * A cursor is advanced when the document at its current iterate is removed.
+ */
+ class HandleDelete : public Base {
+ public:
+ void run() {
+ for( int i = 0; i < 150; ++i ) {
+ client.insert( ns(), BSON( "_id" << i ) );
+ }
+
+ boost::shared_ptr<Cursor> cursor;
+ ClientCursor::Holder clientCursor;
+ ClientCursor::YieldData yieldData;
+
+ {
+ Client::ReadContext ctx( ns() );
+ // The query will utilize the _id index for both the first and second clauses.
+ cursor = NamespaceDetailsTransient::getCursor
+ ( ns(), fromjson( "{$or:[{_id:{$gte:0,$lte:148}},{_id:149}]}" ) );
+ clientCursor.reset( new ClientCursor( QueryOption_NoCursorTimeout, cursor,
+ ns() ) );
+ // Advance to the last iterate of the first clause.
+ ASSERT( cursor->ok() );
+ while( cursor->current()[ "_id" ].number() != 148 ) {
+ ASSERT( cursor->advance() );
+ }
+ clientCursor->prepareToYield( yieldData );
+ }
+
+ // Remove the document at the cursor's current position, which will cause the
+ // cursor to be advanced.
+ client.remove( ns(), BSON( "_id" << 148 ) );
+
+ {
+ Client::ReadContext ctx( ns() );
+ clientCursor->recoverFromYield( yieldData );
+ // Verify that the cursor has another iterate, _id:149, after it is advanced due
+ // to _id:148's removal.
+ ASSERT( cursor->ok() );
+ ASSERT_EQUALS( 149, cursor->current()[ "_id" ].number() );
+ }
+ }
+ };
+
/**
* ClientCursor::aboutToDelete() advances a ClientCursor with a refLoc() matching the
* document to be deleted.
@@ -470,6 +514,7 @@ namespace CursorTests {
add<BtreeCursor::RangeEq>();
add<BtreeCursor::RangeIn>();
add<BtreeCursor::AbortImplicitScan>();
+ add<ClientCursor::HandleDelete>();
add<ClientCursor::AboutToDelete>();
add<ClientCursor::AboutToDeleteDuplicate>();
add<ClientCursor::AboutToDeleteDuplicateNextClause>();
- is depended on by
-
-
- Closed
-
- related to
-
SERVER-6052 Investigate killop in ClientCursor::aboutToDelete()
-
- Closed
-