Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

SERVER-4401 attempt to advance to next or clause if a cursor becomes …

…invalid on yield recovery
  • Loading branch information...
commit 2d5c1aeeb470df3e79b1400a5410b73676d73c97 1 parent b8447af
@astaple astaple authored
View
74 db/queryoptimizercursor.cpp
@@ -116,12 +116,15 @@ namespace mongo {
DiskLoc currLoc() const { return _c ? _c->currLoc() : DiskLoc(); }
BSONObj currKey() const { return _c ? _c->currKey() : BSONObj(); }
virtual bool mayRecordPlan() const {
- return complete() && !stopRequested();
+ return !_yieldRecoveryFailed && complete() && !stopRequested();
}
shared_ptr<Cursor> cursor() const { return _c; }
private:
void mayAdvance() {
- if ( _mustAdvance && _c ) {
+ if ( !_c ) {
+ return;
+ }
+ if ( _mustAdvance ) {
_c->advance();
_mustAdvance = false;
}
@@ -185,36 +188,7 @@ namespace mongo {
return DiskLoc();
}
virtual bool advance() {
- if ( _takeover ) {
- return _takeover->advance();
- }
-
- // Ok to advance if currOp in an error state due to failed yield recovery.
- // This may be the case when advance() is called by recoverFromYield().
- if ( !( _currOp && _currOp->error() ) && !ok() ) {
- return false;
- }
-
- _currOp = 0;
- shared_ptr<QueryOp> op = _mps->nextOp();
- rethrowOnError( op );
-
- QueryOptimizerCursorOp *qocop = dynamic_cast<QueryOptimizerCursorOp*>( op.get() );
- if ( !op->complete() ) {
- // 'qocop' will be valid until we call _mps->nextOp() again.
- _currOp = qocop;
- }
- else if ( op->stopRequested() ) {
- if ( qocop->cursor() ) {
- _takeover.reset( new MultiCursor( _mps,
- qocop->cursor(),
- op->matcher( qocop->cursor() ),
- *op,
- _nscanned - qocop->cursor()->nscanned() ) );
- }
- }
-
- return ok();
+ return _advance( false );
}
virtual BSONObj currKey() const {
if ( _takeover ) {
@@ -256,9 +230,9 @@ namespace mongo {
}
if ( _currOp ) {
_mps->recoverFromYield();
- if ( _currOp->error() ) {
- // See if we can advance to a non error op.
- advance();
+ if ( _currOp->error() || !ok() ) {
+ // Advance to a non error op or a following $or clause if possible.
+ _advance( true );
}
}
}
@@ -308,6 +282,36 @@ namespace mongo {
}
private:
+ bool _advance( bool force ) {
+ if ( _takeover ) {
+ return _takeover->advance();
+ }
+
+ if ( !force && !ok() ) {
+ return false;
+ }
+
+ _currOp = 0;
+ shared_ptr<QueryOp> op = _mps->nextOp();
+ rethrowOnError( op );
+
+ QueryOptimizerCursorOp *qocop = dynamic_cast<QueryOptimizerCursorOp*>( op.get() );
+ if ( !op->complete() ) {
+ // 'qocop' will be valid until we call _mps->nextOp() again.
+ _currOp = qocop;
+ }
+ else if ( op->stopRequested() ) {
+ if ( qocop->cursor() ) {
+ _takeover.reset( new MultiCursor( _mps,
+ qocop->cursor(),
+ op->matcher( qocop->cursor() ),
+ *op,
+ _nscanned - qocop->cursor()->nscanned() ) );
+ }
+ }
+
+ return ok();
+ }
void rethrowOnError( const shared_ptr< QueryOp > &op ) {
// If all plans have erred out, assert.
if ( op->error() ) {
View
92 dbtests/queryoptimizertests.cpp
@@ -1921,6 +1921,33 @@ namespace QueryOptimizerTests {
}
};
+ /** Yield and remove document with $or query. */
+ class YieldRemoveOr : public Base {
+ public:
+ void run() {
+ _cli.insert( ns(), BSON( "_id" << 1 ) );
+ _cli.insert( ns(), BSON( "_id" << 2 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ setQueryOptimizerCursor( BSON( "$or" << BSON_ARRAY( BSON( "_id" << 1 ) << BSON( "_id" << 2 ) ) ) );
+ ASSERT_EQUALS( 1, current().getIntField( "_id" ) );
+ ASSERT( prepareToYield() );
+ }
+
+ _cli.remove( ns(), BSON( "_id" << 1 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ recoverFromYield();
+ ASSERT( ok() );
+ ASSERT_EQUALS( 2, current().getIntField( "_id" ) );
+ }
+ }
+ };
+
/** Yield and overwrite current in capped collection. */
class YieldCappedOverwrite : public Base {
public:
@@ -2074,6 +2101,68 @@ namespace QueryOptimizerTests {
}
};
+ /** Yielding with delete, multiple plans active, and $or clause. */
+ class YieldMultiplePlansDeleteOr : public Base {
+ public:
+ void run() {
+ _cli.insert( ns(), BSON( "_id" << 1 << "a" << 2 ) );
+ _cli.insert( ns(), BSON( "_id" << 2 << "a" << 1 ) );
+ _cli.ensureIndex( ns(), BSON( "a" << 1 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ setQueryOptimizerCursor( BSON( "$or" << BSON_ARRAY( BSON( "_id" << 1 << "a" << 2 ) << BSON( "_id" << 2 << "a" << 1 ) ) ) );
+ ASSERT_EQUALS( 1, current().getIntField( "_id" ) );
+ ASSERT( prepareToYield() );
+ }
+
+ _cli.remove( ns(), BSON( "_id" << 1 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ c()->recoverFromYield();
+ ASSERT( ok() );
+ ASSERT_EQUALS( 2, current().getIntField( "_id" ) );
+ ASSERT( !advance() );
+ ASSERT( !ok() );
+ }
+ }
+ };
+
+ /** Yielding with delete, multiple plans active with advancement to the second, and $or clause. */
+ class YieldMultiplePlansDeleteOrAdvance : public Base {
+ public:
+ void run() {
+ _cli.insert( ns(), BSON( "_id" << 1 << "a" << 2 ) );
+ _cli.insert( ns(), BSON( "_id" << 2 << "a" << 1 ) );
+ _cli.ensureIndex( ns(), BSON( "a" << 1 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ setQueryOptimizerCursor( BSON( "$or" << BSON_ARRAY( BSON( "_id" << 1 << "a" << 2 ) << BSON( "_id" << 2 << "a" << 1 ) ) ) );
+ ASSERT_EQUALS( 1, current().getIntField( "_id" ) );
+ ASSERT( prepareToYield() );
+ c()->advance();
+ ASSERT_EQUALS( 1, current().getIntField( "_id" ) );
+ }
+
+ _cli.remove( ns(), BSON( "_id" << 1 ) );
+
+ {
+ dblock lk;
+ Client::Context ctx( ns() );
+ c()->recoverFromYield();
+ ASSERT( ok() );
+ ASSERT_EQUALS( 2, current().getIntField( "_id" ) );
+ ASSERT( !advance() );
+ ASSERT( !ok() );
+ }
+ }
+ };
+
/** Yielding with multiple plans and capped overwrite. */
class YieldMultiplePlansCappedOverwrite : public Base {
public:
@@ -2703,11 +2792,14 @@ namespace QueryOptimizerTests {
add<QueryOptimizerCursorTests::YieldUpdate>();
add<QueryOptimizerCursorTests::YieldDrop>();
add<QueryOptimizerCursorTests::YieldDropOr>();
+ add<QueryOptimizerCursorTests::YieldRemoveOr>();
add<QueryOptimizerCursorTests::YieldCappedOverwrite>();
add<QueryOptimizerCursorTests::YieldDropIndex>();
add<QueryOptimizerCursorTests::YieldMultiplePlansNoOp>();
add<QueryOptimizerCursorTests::YieldMultiplePlansAdvanceNoOp>();
add<QueryOptimizerCursorTests::YieldMultiplePlansDelete>();
+ add<QueryOptimizerCursorTests::YieldMultiplePlansDeleteOr>();
+ add<QueryOptimizerCursorTests::YieldMultiplePlansDeleteOrAdvance>();
add<QueryOptimizerCursorTests::YieldMultiplePlansCappedOverwrite>();
add<QueryOptimizerCursorTests::YieldMultiplePlansCappedOverwriteManual>();
add<QueryOptimizerCursorTests::YieldMultiplePlansCappedOverwriteManual2>();
View
27 jstests/distinct3.js
@@ -0,0 +1,27 @@
+// Yield and delete test case for query optimizer cursor.
+
+t = db.jstests_distinct3;
+t.drop();
+
+t.ensureIndex({a:1});
+t.ensureIndex({b:1});
+
+for( i = 0; i < 50; ++i ) {
+ for( j = 0; j < 20; ++j ) {
+ t.save({a:i,c:i,d:j});
+ }
+}
+for( i = 0; i < 1000; ++i ) {
+ t.save({b:i,c:i+50});
+}
+db.getLastError();
+
+// The idea here is to try and remove the last match for the {a:1} index scan while distinct is yielding.
+p = startParallelShell( 'for( i = 0; i < 2500; ++i ) { db.jstests_distinct3.remove({a:49}); for( j = 0; j < 20; ++j ) { db.jstests_distinct3.save({a:49,c:49,d:j}) } }' );
+
+for( i = 0; i < 100; ++i ) {
+ count = t.distinct( 'c', {$or:[{a:{$gte:0},d:0},{b:{$gte:0}}]} ).length;
+ assert.gt( count, 1000 );
+}
+
+p();
Please sign in to comment.
Something went wrong with that request. Please try again.