diff --git a/db/btree.cpp b/db/btree.cpp index 1fd0cefac57..ec2b28537de 100644 --- a/db/btree.cpp +++ b/db/btree.cpp @@ -866,34 +866,48 @@ void BtreeCursor::noteLocation() { } } -int clctr = 0; +/* Since the last noteLocation(), our key may have moved around, and that old cached + information may thus be stale and wrong (although often it is right). We check + that here; if we have moved, we have to search back for where we were at. -/* see if things moved around (deletes, splits, inserts) */ + i.e., after operations on the index, the BtreeCursor's cached location info may + be invalid. This function ensures validity, so you should call it before using + the cursor if other writers have used the database since the last noteLocation + call. +*/ void BtreeCursor::checkLocation() { - try { + { if( eof() ) return; - BtreeBucket *b = bucket.btree(); + BtreeBucket *b = bucket.btree(); + + assert( !keyAtKeyOfs.isEmpty() ); + + // Note keyAt() returns an empty JSObj if keyOfs is now out of range, + // which is possible as keys may have been deleted. if( b->keyAt(keyOfs).woEqual(keyAtKeyOfs) && b->k(keyOfs).recordLoc == locAtKeyOfs ) { - if( !b->k(keyOfs).isUsed() ) - checkUnused(); - return; + if( !b->k(keyOfs).isUsed() ) { + /* we were deleted but still exist as an unused + marker key. advance. + */ + checkUnused(); + } + return; } } - catch( AssertionException ) { - cout << "Caught exception in checkLocation(), that's maybe ok" << endl; - } + + /* normally we don't get to here. when we do, old position is no longer + valid and we must refind where we left off (which is expensive) + */ bool found; DiskLoc bold = bucket; -/* TODO: Switch to keep indexdetails and do idx.head! */ - /* didn't find, check from the top */ + /* TODO: Switch to keep indexdetails and do idx.head! */ DiskLoc head = bold.btree()->getHead(bold); bucket = head.btree()->locate(head, keyAtKeyOfs, keyOfs, found, locAtKeyOfs, direction); - if( clctr++ % 128 == 0 ) - cout << " key seems to have moved in the index, refinding. found:" << found << endl; + RARELY cout << " key seems to have moved in the index, refinding. found:" << found << endl; if( found ) checkUnused(); } diff --git a/db/btree.h b/db/btree.h index 063196f67b4..e719d8e8965 100644 --- a/db/btree.h +++ b/db/btree.h @@ -17,8 +17,16 @@ struct _KeyNode { unsigned short _kdo; void setKeyDataOfs(short s) { _kdo = s; assert(s>=0); } void setKeyDataOfsSavingUse(short s) { _kdo = s; assert(s>=0); } - void setUnused() { recordLoc.GETOFS() |= 1; } -// void setUsed() { _kdo &= 0x7fff; } + void setUnused() { + /* setting ofs to 1 is the sentinel. setInvalid sets the + fileno, that is defensive code. + */ + recordLoc.setInvalid(); + recordLoc.GETOFS() = 1; + } + /* & 1 for backward compatibility. can be made "== 1" later when we increment + the db data version + */ int isUnused() { return (recordLoc.getOfs() & 1); } int isUsed() { return !isUnused(); } }; diff --git a/db/jsobj.h b/db/jsobj.h index 76b97c33b83..c3dc8b8e960 100644 --- a/db/jsobj.h +++ b/db/jsobj.h @@ -170,16 +170,7 @@ class JSObj { friend class JSElemIter; class Details { public: - ~Details() { - - assert(refCount <= 0); - - // note that the assert above protects owned() from lying to us. If the assert() goes, fix this - if (owned()) { - free((void *)_objdata); - } - _objdata = 0; - } + ~Details() { _objdata = 0; } const char *_objdata; int _objsize; int refCount; // -1 == don't free @@ -233,8 +224,8 @@ public: JSObj extractFields(JSObj pattern, JSObjBuilder& b); const char *objdata() const { return details->_objdata; } - int objsize() const { return details->_objsize; } // includes the embedded size field - bool isEmpty() const { return details == 0 || objsize() <= 5; } + int objsize() const { return details ? details->_objsize : 0; } // includes the embedded size field + bool isEmpty() const { return objsize() <= 5; } /* this is broken if elements aren't in the same order. */ bool operator<(const JSObj& r) const { return woCompare(r) < 0; } @@ -244,7 +235,9 @@ public: */ int woCompare(const JSObj& r) const; bool woEqual(const JSObj& r) const { - return objsize()==r.objsize() && memcmp(objdata(),r.objdata(),objsize())==0; + int os = objsize(); + return os == r.objsize() && + (os == 0 || memcmp(objdata(),r.objdata(),os)==0); } bool operator==(const JSObj& r) const { return this->woEqual(r); diff --git a/db/pdfile.cpp b/db/pdfile.cpp index 6563e41ae77..cc61ce903d7 100644 --- a/db/pdfile.cpp +++ b/db/pdfile.cpp @@ -717,8 +717,6 @@ void DataFileMgr::deleteRecord(const char *ns, Record *todelete, const DiskLoc& { dassert( todelete == dl.rec() ); - int tempextofs = todelete->extentOfs; - NamespaceDetails* d = nsdetails(ns); if( d->capped && !cappedOK ) { cout << "failing remove on a capped ns " << ns << endl; diff --git a/db/query.cpp b/db/query.cpp index 42b8a4a4ae6..fef59074a0c 100644 --- a/db/query.cpp +++ b/db/query.cpp @@ -150,7 +150,7 @@ auto_ptr getIndexCursor(const char *ns, JSObj& query, JSObj& order, bool } fail: - DEV cout << "getIndexCursor fail" << endl; + DEV cout << "getIndexCursor fail " << ns << '\n'; return auto_ptr(); }