From 3055ab6d1de96207c9e372d0808ae8d75a7cd2b3 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 11 Mar 2009 17:05:50 -0400 Subject: [PATCH] Clean up tailable cursor code --- db/btree.h | 3 ++ db/clientcursor.cpp | 14 +++------ db/cursor.h | 71 +++++++++++++++++------------------------- db/query.cpp | 21 +++++++------ dbtests/querytests.cpp | 40 ++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 62 deletions(-) diff --git a/db/btree.h b/db/btree.h index e8f0b915f1f..d49db2a8729 100644 --- a/db/btree.h +++ b/db/btree.h @@ -248,6 +248,9 @@ namespace mongo { virtual DiskLoc currLoc() { return !bucket.isNull() ? _currKeyNode().recordLoc : DiskLoc(); } + virtual DiskLoc refLoc() { + return currLoc(); + } virtual Record* _current() { return currLoc().rec(); } diff --git a/db/clientcursor.cpp b/db/clientcursor.cpp index 1dc78f23c27..cc7f3aea190 100644 --- a/db/clientcursor.cpp +++ b/db/clientcursor.cpp @@ -119,25 +119,19 @@ namespace mongo { i != toAdvance.end(); ++i ) { Cursor *c = (*i)->c.get(); - DiskLoc tmp1 = c->currLoc(); + DiskLoc tmp1 = c->refLoc(); if ( tmp1 != dl ) { /* this might indicate a failure to call ClientCursor::updateLocation() */ problem() << "warning: cursor loc does not match byLoc position!" << endl; } c->checkLocation(); - if ( c->tailing() ) { - DEV out() << "killing cursor as we would have to advance it and it is tailable" << endl; - delete *i; - continue; - } c->advance(); - DiskLoc newLoc = c->currLoc(); - if ( newLoc.isNull() ) { + if ( c->eof() ) { // advanced to end -- delete cursor delete *i; } else { - wassert( newLoc != dl ); + wassert( c->refLoc() != dl ); (*i)->updateLocation(); } } @@ -158,7 +152,7 @@ namespace mongo { */ void ClientCursor::updateLocation() { assert( cursorid ); - DiskLoc cl = c->currLoc(); + DiskLoc cl = c->refLoc(); if ( lastLoc() == cl ) { //log() << "info: lastloc==curloc " << ns << '\n'; return; diff --git a/db/cursor.h b/db/cursor.h index 063cdfe267e..f191c004682 100644 --- a/db/cursor.h +++ b/db/cursor.h @@ -53,24 +53,19 @@ namespace mongo { virtual DiskLoc currLoc() = 0; virtual bool advance() = 0; /*true=ok*/ virtual BSONObj currKey() const { return emptyObj; } + // DiskLoc the cursor requires for continued operation. Before this + // DiskLoc is deleted, the cursor must be incremented or destroyed. + virtual DiskLoc refLoc() = 0; /* Implement these if you want the cursor to be "tailable" */ - /* tailable(): if true, cursor has tailable capability AND - the user requested use of those semantics. */ + + /* Request that the cursor starts tailing after advancing past last record. */ + /* The implementation may or may not honor this request. */ + virtual void setTailable() {} + /* indicates if tailing is enabled. */ virtual bool tailable() { return false; } - /* indicates we should mark where we are and go into tail mode. */ - virtual void setAtTail() { - assert(false); - } - /* you must call tailResume before reusing the cursor */ - virtual void tailResume() { } - /* indicates ifi we are actively tailing. once it goes active, - this should return treu even after tailResume(). */ - virtual bool tailing() { - return false; - } virtual void aboutToDeleteBucket(const DiskLoc& b) { } @@ -107,6 +102,7 @@ namespace mongo { virtual BSONObj prettyStartKey() const { return emptyObj; } virtual BSONObj prettyEndKey() const { return emptyObj; } + }; class AdvanceStrategy { @@ -125,12 +121,10 @@ namespace mongo { AdvanceStrategy *s; private: - // for tailing: - enum State { Normal, TailPoint, TailResumed } state; + bool tailable_; void init() { - state = Normal; + tailable_ = false; } - public: bool ok() { return !curr.isNull(); @@ -147,14 +141,22 @@ namespace mongo { virtual DiskLoc currLoc() { return curr; } - + virtual DiskLoc refLoc() { + return curr.isNull() ? last : curr; + } + bool advance() { checkForInterrupt(); - if ( eof() ) - return false; - _current(); - last = curr; - curr = s->next( curr ); + if ( eof() ) { + if ( tailable_ && !last.isNull() ) { + curr = s->next( last ); + } else { + return false; + } + } else { + last = curr; + curr = s->next( curr ); + } return ok(); } @@ -167,27 +169,12 @@ namespace mongo { virtual string toString() { return "BasicCursor"; } - - virtual void tailResume() { - if ( state == TailPoint ) { - state = TailResumed; - advance(); - } - } - virtual void setAtTail() { - assert( state != TailPoint ); - assert( curr.isNull() ); - assert( !last.isNull() ); - curr = last; - last.Null(); - state = TailPoint; + virtual void setTailable() { + if ( !curr.isNull() || !last.isNull() ) + tailable_ = true; } virtual bool tailable() { - // to go into tail mode we need a non-null point of reference for resumption - return !last.isNull(); - } - virtual bool tailing() { - return state != Normal; + return tailable_; } }; diff --git a/db/query.cpp b/db/query.cpp index 95f068ce70f..1a0dee214b1 100644 --- a/db/query.cpp +++ b/db/query.cpp @@ -511,7 +511,7 @@ namespace mongo { int n = 0; if ( !cc ) { - DEV log() << "getMore: cursorid not found " << ns << " " << cursorid << endl; + log() << "getMore: cursorid not found " << ns << " " << cursorid << endl; cursorid = 0; resultFlags = QueryResult::ResultFlag_CursorNotFound; } @@ -519,14 +519,14 @@ namespace mongo { start = cc->pos; Cursor *c = cc->c.get(); c->checkLocation(); - c->tailResume(); while ( 1 ) { if ( !c->ok() ) { - if ( c->tailing() ) { - c->setAtTail(); + if ( c->tailable() ) { + if ( c->advance() ) { + continue; + } break; } - DEV log() << " getmore: last batch, erasing cursor " << cursorid << endl; bool ok = ClientCursor::erase(cursorid); assert(ok); cursorid = 0; @@ -549,8 +549,6 @@ namespace mongo { if ( (ntoreturn>0 && (n >= ntoreturn || b.len() > MaxBytesToReturnToClientAtOnce)) || (ntoreturn==0 && b.len()>1*1024*1024) ) { c->advance(); - if ( c->tailing() && !c->ok() ) - c->setAtTail(); cc->pos += n; //cc->updateLocation(); break; @@ -770,8 +768,11 @@ namespace mongo { } else if ( ordering_ ) { so_->fill(b_, filter_, n_); } - else if ( !saveClientCursor_ && !c_->ok() && (queryOptions_ & Option_CursorTailable) && c_->tailable() ) { - c_->setAtTail(); + if ( ( queryOptions_ & Option_CursorTailable ) && ntoreturn_ != 1 ) { + c_->setTailable(); + } + // If the tailing request succeeded. + if ( c_->tailable() ) { saveClientCursor_ = true; } setComplete(); @@ -932,7 +933,7 @@ namespace mongo { cc->filter = filter; cc->originalMessage = m; cc->updateLocation(); - if ( cc->c->tailing() ) + if ( !cc->c->ok() && cc->c->tailable() ) DEV out() << " query has no more but tailable, cursorid: " << cursorid << endl; else DEV out() << " query has more, cursorid: " << cursorid << endl; diff --git a/dbtests/querytests.cpp b/dbtests/querytests.cpp index d52b6e90be4..3bd6334ed3e 100644 --- a/dbtests/querytests.cpp +++ b/dbtests/querytests.cpp @@ -248,12 +248,50 @@ namespace QueryTests { insert( ns, BSON( "a" << 1 ) ); insert( ns, BSON( "a" << 2 ) ); auto_ptr< DBClientCursor > c = client().query( ns, QUERY( "a" << GT << 0 ).hint( BSON( "$natural" << 1 ) ), 1, 0, 0, Option_CursorTailable ); + // If only one result requested, a cursor is not saved. ASSERT_EQUALS( 0, c->getCursorId() ); ASSERT( c->more() ); ASSERT_EQUALS( 1, c->next().getIntField( "a" ) ); } }; + class TailNotAtEnd : public ClientBase { + public: + ~TailNotAtEnd() { + client().dropCollection( "querytests.TailNotAtEnd" ); + } + void run() { + const char *ns = "querytests.TailNotAtEnd"; + insert( ns, BSON( "a" << 0 ) ); + insert( ns, BSON( "a" << 1 ) ); + insert( ns, BSON( "a" << 2 ) ); + auto_ptr< DBClientCursor > c = client().query( ns, Query().hint( BSON( "$natural" << 1 ) ), 2, 0, 0, Option_CursorTailable ); + ASSERT( 0 != c->getCursorId() ); + while( c->more() ) + c->next(); + ASSERT( 0 != c->getCursorId() ); + insert( ns, BSON( "a" << 3 ) ); + insert( ns, BSON( "a" << 4 ) ); + insert( ns, BSON( "a" << 5 ) ); + insert( ns, BSON( "a" << 6 ) ); + ASSERT( c->more() ); + ASSERT_EQUALS( 3, c->next().getIntField( "a" ) ); + } + }; + + class EmptyTail : public ClientBase { + public: + ~EmptyTail() { + client().dropCollection( "querytests.EmptyTail" ); + } + void run() { + const char *ns = "querytests.EmptyTail"; + ASSERT_EQUALS( 0, client().query( ns, Query().hint( BSON( "$natural" << 1 ) ), 2, 0, 0, Option_CursorTailable )->getCursorId() ); + insert( ns, BSON( "a" << 0 ) ); + ASSERT( 0 != client().query( ns, QUERY( "a" << 1 ).hint( BSON( "$natural" << 1 ) ), 2, 0, 0, Option_CursorTailable )->getCursorId() ); + } + }; + class All : public UnitTest::Suite { public: All() { @@ -271,6 +309,8 @@ namespace QueryTests { add< BoundedKey >(); add< GetMore >(); add< ReturnOneOfManyAndTail >(); + add< TailNotAtEnd >(); + add< EmptyTail >(); } };