Skip to content

Commit 40020f6

Browse files
committed
SERVER-1752 Within BtreeCursor::currentMatches(), filter keys that do not match the FieldRangeVector's index bounds.
1 parent 2b049cb commit 40020f6

File tree

4 files changed

+117
-15
lines changed

4 files changed

+117
-15
lines changed

src/mongo/db/btree.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,8 @@ namespace mongo {
10971097

10981098
virtual CoveredIndexMatcher *matcher() const { return _matcher.get(); }
10991099

1100+
virtual bool currentMatches( MatchDetails* details = 0 );
1101+
11001102
virtual void setMatcher( shared_ptr< CoveredIndexMatcher > matcher ) { _matcher = matcher; }
11011103

11021104
virtual const Projection::KeyOnly *keyFieldsOnly() const { return _keyFieldsOnly.get(); }
@@ -1122,7 +1124,16 @@ namespace mongo {
11221124
virtual bool skipUnusedKeys() = 0;
11231125

11241126
bool skipOutOfRangeKeysAndCheckEnd();
1127+
1128+
/**
1129+
* Attempt to locate the next btree key matching _bounds. This may mean advancing to the
1130+
* next successive key in the btree, or skipping to a new position in the btree. If an
1131+
* internal iteration cutoff is reached before a matching key is found, then the search for
1132+
* a matching key will be aborted, leaving the cursor pointing at a key that is not within
1133+
* bounds.
1134+
*/
11251135
void skipAndCheck();
1136+
11261137
void checkEnd();
11271138

11281139
/** selective audits on construction */
@@ -1159,6 +1170,10 @@ namespace mongo {
11591170
DiskLoc locAtKeyOfs;
11601171
shared_ptr< FieldRangeVector > _bounds;
11611172
auto_ptr< FieldRangeVectorIterator > _boundsIterator;
1173+
bool _boundsMustMatch; // If iteration is aborted before a key matching _bounds is
1174+
// identified, the cursor may be left pointing at a key that is not
1175+
// within bounds (_bounds->matchesKey( currKey() ) may be false).
1176+
// _boundsMustMatch will be set to false accordingly.
11621177
shared_ptr< CoveredIndexMatcher > _matcher;
11631178
shared_ptr<Projection::KeyOnly> _keyFieldsOnly;
11641179
bool _independentFieldRanges;

src/mongo/db/btreecursor.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,13 @@ namespace mongo {
239239
return c.release();
240240
}
241241

242-
BtreeCursor::BtreeCursor( NamespaceDetails* nsd , int theIndexNo, const IndexDetails& id )
243-
: d( nsd ) , idxNo( theIndexNo ) , indexDetails( id ) , _ordering(Ordering::make(BSONObj())){
244-
_nscanned = 0;
242+
BtreeCursor::BtreeCursor( NamespaceDetails* nsd, int theIndexNo, const IndexDetails& id ) :
243+
d( nsd ),
244+
idxNo( theIndexNo ),
245+
indexDetails( id ),
246+
_ordering( Ordering::make( BSONObj() ) ),
247+
_boundsMustMatch( true ),
248+
_nscanned() {
245249
}
246250

247251
void BtreeCursor::_finishConstructorInit() {
@@ -304,8 +308,17 @@ namespace mongo {
304308
break;
305309
}
306310
do {
311+
// If nscanned is increased by more than 20 before a matching key is found, abort
312+
// skipping through the btree to find a matching key. This iteration cutoff
313+
// prevents unbounded internal iteration within BtreeCursor::init() and
314+
// BtreeCursor::advance() (the callers of skipAndCheck()). See SERVER-3448.
307315
if ( _nscanned > startNscanned + 20 ) {
308316
skipUnusedKeys();
317+
// If iteration is aborted before a key matching _bounds is identified, the
318+
// cursor may be left pointing at a key that is not within bounds
319+
// (_bounds->matchesKey( currKey() ) may be false). Set _boundsMustMatch to
320+
// false accordingly.
321+
_boundsMustMatch = false;
309322
return;
310323
}
311324
} while( skipOutOfRangeKeysAndCheckEnd() );
@@ -357,6 +370,9 @@ namespace mongo {
357370
}
358371

359372
bool BtreeCursor::advance() {
373+
// Reset this flag at the start of a new iteration.
374+
_boundsMustMatch = true;
375+
360376
killCurrentOp.checkForInterrupt();
361377
if ( bucket.isNull() )
362378
return false;
@@ -398,7 +414,17 @@ namespace mongo {
398414
else {
399415
return _bounds->obj();
400416
}
401-
}
417+
}
418+
419+
bool BtreeCursor::currentMatches( MatchDetails* details ) {
420+
// If currKey() might not match the specified _bounds, check whether or not it does.
421+
if ( !_boundsMustMatch && _bounds && !_bounds->matchesKey( currKey() ) ) {
422+
// If the key does not match _bounds, it does not match the query.
423+
return false;
424+
}
425+
// Forward to the base class implementation, which may utilize a Matcher.
426+
return Cursor::currentMatches( details );
427+
}
402428

403429
/* ----------------------------------------------------------------------------- */
404430

src/mongo/db/queryutil.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,19 @@ namespace mongo {
637637
* FieldRangeVector. This function is used for $or clause deduping.
638638
*/
639639
bool matches( const BSONObj &obj ) const;
640-
640+
641+
/**
642+
* @return true if all values in the provided index key are contained within the field
643+
* ranges of their respective fields in this FieldRangeVector.
644+
*
645+
* For example, given a query { a:3, b:4 } and index { a:1, b:1 }, the FieldRangeVector is
646+
* [ [[ 3, 3 ]], [[ 4, 4 ]] ], consisting of field range [[ 3, 3 ]] on field 'a' and
647+
* [[ 4, 4 ]] on field 'b'. The index key { '':3, '':4 } matches, but the index key
648+
* { '':3, '':5 } does not match because the value 5 in the second field is not contained in
649+
* the field range [[ 4, 4 ]] for field 'b'.
650+
*/
651+
bool matchesKey( const BSONObj& key ) const;
652+
641653
/**
642654
* @return first key of 'obj' that would be encountered by a forward
643655
* index scan using this FieldRangeVector, BSONObj() if no such key.
@@ -649,7 +661,6 @@ namespace mongo {
649661
private:
650662
int matchingLowElement( const BSONElement &e, int i, bool direction, bool &lowEquality ) const;
651663
bool matchesElement( const BSONElement &e, int i, bool direction ) const;
652-
bool matchesKey( const BSONObj &key ) const;
653664
vector<FieldRange> _ranges;
654665
const IndexSpec _indexSpec;
655666
int _direction;

src/mongo/dbtests/cursortests.cpp

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -260,26 +260,76 @@ namespace CursorTests {
260260
}
261261
virtual BSONObj idx() const { return BSON( "a" << 1 << "b" << 1 ); }
262262
};
263-
263+
264+
/**
265+
* BtreeCursor::advance() may skip to new btree positions multiple times. A cutoff (tested
266+
* here) has been implemented to avoid excessive iteration in such cases. See SERVER-3448.
267+
*/
264268
class AbortImplicitScan : public Base {
265269
public:
266270
void run() {
271+
// Set up a compound index with some data.
267272
IndexSpec idx( BSON( "a" << 1 << "b" << 1 ) );
268273
_c.ensureIndex( ns(), idx.keyPattern );
269274
for( int i = 0; i < 300; ++i ) {
270-
_c.insert( ns(), BSON( "a" << i << "b" << 5 ) );
275+
_c.insert( ns(), BSON( "a" << i << "b" << i ) );
271276
}
272-
FieldRangeSet frs( ns(), BSON( "b" << 3 ), true, true );
277+
_c.insert( ns(), BSON( "a" << 300 << "b" << 30 ) );
278+
279+
// Set up a cursor on the { a:1, b:1 } index, the same cursor that would be created
280+
// for the query { b:30 }. Because this query has no constraint on 'a' (the
281+
// first field of the compound index), the cursor will examine every distinct value
282+
// of 'a' in the index and check for an index key with that value for 'a' and 'b'
283+
// equal to 30.
284+
FieldRangeSet frs( ns(), BSON( "b" << 30 ), true, true );
273285
boost::shared_ptr<FieldRangeVector> frv( new FieldRangeVector( frs, idx, 1 ) );
274286
Client::WriteContext ctx( ns() );
275-
scoped_ptr<BtreeCursor> c( BtreeCursor::make( nsdetails( ns() ), nsdetails( ns() )->idx(1), frv, 1 ) );
276-
long long initialNscanned = c->nscanned();
277-
ASSERT( initialNscanned < 200 );
287+
scoped_ptr<BtreeCursor> c( BtreeCursor::make( nsdetails( ns() ),
288+
nsdetails( ns() )->idx(1),
289+
frv,
290+
1 ) );
291+
292+
// BtreeCursor::init() and BtreeCursor::advance() attempt to advance the cursor to
293+
// the next matching key, which may entail examining many successive distinct values
294+
// of 'a' having no index key where b equals 30. To prevent excessive iteration
295+
// within init() and advance(), examining distinct 'a' values is aborted once an
296+
// nscanned cutoff is reached. We test here that this cutoff is applied, and that
297+
// if it is applied before a matching key is found, then
298+
// BtreeCursor::currentMatches() returns false appropriately.
299+
278300
ASSERT( c->ok() );
279-
c->advance();
280-
ASSERT( c->nscanned() > initialNscanned );
301+
// The starting iterate found by BtreeCursor::init() does not match. This is a key
302+
// before the {'':30,'':30} key, because init() is aborted prematurely.
303+
ASSERT( !c->currentMatches() );
304+
// And init() stopped iterating before scanning the whole btree (with ~300 keys).
281305
ASSERT( c->nscanned() < 200 );
282-
ASSERT( c->ok() );
306+
307+
ASSERT( c->advance() );
308+
// The next iterate matches (this is the {'':30,'':30} key).
309+
ASSERT( c->currentMatches() );
310+
311+
int oldNscanned = c->nscanned();
312+
ASSERT( c->advance() );
313+
// Check that nscanned has increased ...
314+
ASSERT( c->nscanned() > oldNscanned );
315+
// ... but that advance() stopped iterating before the whole btree (with ~300 keys)
316+
// was scanned.
317+
ASSERT( c->nscanned() < 200 );
318+
// Because advance() is aborted prematurely, the current iterate does not match.
319+
ASSERT( !c->currentMatches() );
320+
321+
// Iterate through the remainder of the btree.
322+
bool foundLastMatch = false;
323+
while( c->advance() ) {
324+
bool bMatches = ( c->current()[ "b" ].number() == 30 );
325+
// The current iterate only matches if it has the proper 'b' value.
326+
ASSERT_EQUALS( bMatches, c->currentMatches() );
327+
if ( bMatches ) {
328+
foundLastMatch = true;
329+
}
330+
}
331+
// Check that the final match, on key {'':300,'':30}, is found.
332+
ASSERT( foundLastMatch );
283333
}
284334
};
285335

0 commit comments

Comments
 (0)