SERVER-7289 add inexpensive tests to validate command

Checks added to the validate command:
1) The 'xprev' pointer in each extent in the extent list should point
   to the previous extent, or be null for the first extent
2) The 'lastExtent' pointer in NamespaceDetails should point to the last extent
3) If an exception is thrown while counting extents, identify the failing extent
4) If an exception is thrown while counting extents, report the count
5) If an exception is thrown while counting extents, the first extent should be
   validated and the check for 'xprev' in the first extent being null should be done
6) If an invalid signature is found in an extent, report the value found and
   identify the extent
7) The "self-pointer" 'myLoc' in an extent should be validated
8) The extent size should be compared with Extent::minSize(), not zero

3 files changed, 105 insertions, 35 deletions

diff --git a/src/mongo/db/dbcommands_admin.cpp b/src/mongo/db/dbcommands_admin.cpp
index 2bedc168264..804926f43c5 100644
--- a/src/mongo/db/dbcommands_admin.cpp
+++ b/src/mongo/db/dbcommands_admin.cpp
@@ -178,7 +178,10 @@ namespace mongo {
         }
 
     private:
-        void validateNS(const char *ns, NamespaceDetails *d, const BSONObj& cmdObj, BSONObjBuilder& result) {
+        void validateNS(const char *ns,
+                        NamespaceDetails *d,
+                        const BSONObj& cmdObj,
+                        BSONObjBuilder& result) {
             const bool full = cmdObj["full"].trueValue();
             const bool scanData = full || cmdObj["scandata"].trueValue();
 
@@ -189,60 +192,109 @@ namespace mongo {
                 result.appendNumber("max", d->maxCappedDocs());
             }
 
-            result.append("firstExtent", str::stream() << d->firstExtent.toString() << " ns:" << d->firstExtent.ext()->nsDiagnostic.toString());
-            result.append( "lastExtent", str::stream() <<  d->lastExtent.toString() << " ns:" <<  d->lastExtent.ext()->nsDiagnostic.toString());
-            
-            BSONArrayBuilder extentData;
+            result.append("firstExtent", str::stream() << d->firstExtent.toString()
+                                    << " ns:" << d->firstExtent.ext()->nsDiagnostic.toString());
+            result.append( "lastExtent", str::stream() <<  d->lastExtent.toString()
+                                    << " ns:" <<  d->lastExtent.ext()->nsDiagnostic.toString());
 
+            BSONArrayBuilder extentData;
+            int extentCount = 0;
             try {
                 d->firstExtent.ext()->assertOk();
                 d->lastExtent.ext()->assertOk();
 
-                DiskLoc el = d->firstExtent;
-                int ne = 0;
-                while( !el.isNull() ) {
-                    Extent *e = el.ext();
-                    e->assertOk();
-                    if ( full )
-                        extentData << e->dump();
-                    if (!e->validates(el, &errors)) {
+                DiskLoc extentDiskLoc = d->firstExtent;
+                while (!extentDiskLoc.isNull()) {
+                    Extent* thisExtent = extentDiskLoc.ext();
+                    if (full) {
+                        extentData << thisExtent->dump();
+                    }
+                    if (!thisExtent->validates(extentDiskLoc, &errors)) {
+                        valid = false;
+                    }
+                    DiskLoc nextDiskLoc = thisExtent->xnext;
+                    if (extentCount > 0 && !nextDiskLoc.isNull()
+                                        &&  nextDiskLoc.ext()->xprev != extentDiskLoc) {
+                        StringBuilder sb;
+                        sb << "'xprev' pointer " << nextDiskLoc.ext()->xprev.toString()
+                           << " in extent " << nextDiskLoc.toString()
+                           << " does not point to extent " << extentDiskLoc.toString();
+                        errors << sb.str();
                         valid = false;
                     }
-                    el = e->xnext;
-                    ne++;
+                    if (nextDiskLoc.isNull() && extentDiskLoc != d->lastExtent) {
+                        StringBuilder sb;
+                        sb << "'lastExtent' pointer " << d->lastExtent.toString()
+                           << " does not point to last extent in list " << extentDiskLoc.toString();
+                        errors << sb.str();
+                        valid = false;
+                    }
+                    extentDiskLoc = nextDiskLoc;
+                    extentCount++;
                     killCurrentOp.checkForInterrupt();
                 }
-                result.append("extentCount", ne);
             }
-            catch (...) {
-                valid=false;
-                errors << "extent asserted";
+            catch (const DBException& e) {
+                StringBuilder sb;
+                sb << "exception validating extent " << extentCount
+                   << ": " << e.what();
+                errors << sb.str();
+                valid = false;
             }
+            result.append("extentCount", extentCount);
 
             if ( full )
                 result.appendArray( "extents" , extentData.arr() );
 
-            
             result.appendNumber("datasize", d->stats.datasize);
             result.appendNumber("nrecords", d->stats.nrecords);
             result.appendNumber("lastExtentSize", d->lastExtentSize);
             result.appendNumber("padding", d->paddingFactor());
-            
 
             try {
 
+                bool testingLastExtent = false;
                 try {
-                    result.append("firstExtentDetails", d->firstExtent.ext()->dump());
-                    if (!d->firstExtent.ext()->xprev.isNull()) {
-                        StringBuilder sb;
-                        sb << "'xprev' pointer in first extent " << d->firstExtent.toString()
-                           << " is not null";
-                        errors << sb.str();
+                    if (d->firstExtent.isNull()) {
+                        errors << "'firstExtent' pointer is null";
                         valid=false;
                     }
+                    else {
+                        result.append("firstExtentDetails", d->firstExtent.ext()->dump());
+                        if (!d->firstExtent.ext()->xprev.isNull()) {
+                            StringBuilder sb;
+                            sb << "'xprev' pointer in 'firstExtent' " << d->firstExtent.toString()
+                               << " is " << d->firstExtent.ext()->xprev.toString()
+                               << ", should be null";
+                            errors << sb.str();
+                            valid=false;
+                        }
+                    }
+                    testingLastExtent = true;
+                    if (d->lastExtent.isNull()) {
+                        errors << "'lastExtent' pointer is null";
+                        valid=false;
+                    }
+                    else {
+                        if (d->firstExtent != d->lastExtent) {
+                            result.append("lastExtentDetails", d->lastExtent.ext()->dump());
+                            if (!d->lastExtent.ext()->xnext.isNull()) {
+                                StringBuilder sb;
+                                sb << "'xnext' pointer in 'lastExtent' " << d->lastExtent.toString()
+                                   << " is " << d->lastExtent.ext()->xnext.toString()
+                                   << ", should be null";
+                                errors << sb.str();
+                                valid = false;
+                            }
+                        }
+                    }
                 }
-                catch (...) {
-                    errors << "exception firstextent";
+                catch (const DBException& e) {
+                    StringBuilder sb;
+                    sb << "exception processing '"
+                       << (testingLastExtent ? "lastExtent" : "firstExtent")
+                       << "': " << e.what();
+                    errors << sb.str();
                     valid = false;
                 }
 
diff --git a/src/mongo/db/pdfile.cpp b/src/mongo/db/pdfile.cpp
index 3967e158e6d..581ae3ca888 100644
--- a/src/mongo/db/pdfile.cpp
+++ b/src/mongo/db/pdfile.cpp
@@ -659,8 +659,14 @@ namespace mongo {
     }
 
     DiskLoc Extent::_reuse(const char *nsname, bool capped) {
-        LOG(3) << "reset extent was:" << nsDiagnostic.toString() << " now:" << nsname << '\n';
-        massert( 10360 ,  "Extent::reset bad magic value", magic == 0x41424344 );
+        LOG(3) << "_reuse extent was:" << nsDiagnostic.toString() << " now:" << nsname << endl;
+        if (magic != extentSignature) {
+            StringBuilder sb;
+            sb << "bad extent signature " << toHex(&magic, 4)
+               << " for namespace '" << nsDiagnostic.toString()
+               << "' found in Extent::_reuse";
+            msgasserted(10360, sb.str());
+        }
         nsDiagnostic = nsname;
         markEmpty();
 
@@ -680,7 +686,7 @@ namespace mongo {
 
     /* assumes already zeroed -- insufficient for block 'reuse' perhaps */
     DiskLoc Extent::init(const char *nsname, int _length, int _fileNo, int _offset, bool capped) {
-        magic = 0x41424344;
+        magic = extentSignature;
         myLoc.set(_fileNo, _offset);
         xnext.Null();
         xprev.Null();
@@ -702,6 +708,15 @@ namespace mongo {
 
         bool Extent::validates(const DiskLoc diskLoc, BSONArrayBuilder* errors) {
             bool extentOk = true;
+            if (magic != extentSignature) {
+                if (errors) {
+                    StringBuilder sb;
+                    sb << "bad extent signature " << toHex(&magic, 4)
+                       << " in extent " << diskLoc.toString();
+                    *errors << sb.str();
+                }
+                extentOk = false;
+            }
             if (myLoc != diskLoc) {
                 if (errors) {
                     StringBuilder sb;
@@ -728,10 +743,12 @@ namespace mongo {
                 }
                 extentOk = false;
             }
-            if (length < 0) {
+            if (length < minSize()) {
                 if (errors) {
                     StringBuilder sb;
-                    sb << "negative length extent " << diskLoc.toString();
+                    sb << "length of extent " << diskLoc.toString()
+                       << " is " << length
+                       << ", which is less than minimum length of " << minSize();
                     *errors << sb.str();
                 }
                 extentOk = false;
diff --git a/src/mongo/db/pdfile.h b/src/mongo/db/pdfile.h
index af224bac8da..20afa4353c4 100644
--- a/src/mongo/db/pdfile.h
+++ b/src/mongo/db/pdfile.h
@@ -314,6 +314,7 @@ namespace mongo {
     */
     class Extent {
     public:
+        enum { extentSignature = 0x41424344 };
         unsigned magic;
         DiskLoc myLoc;
         DiskLoc xnext, xprev; /* next/prev extent for this namespace */
@@ -353,7 +354,7 @@ namespace mongo {
         /* like init(), but for a reuse case */
         DiskLoc reuse(const char *nsname, bool newUseIsAsCapped);
 
-        bool isOk() const { return magic == 0x41424344; }
+        bool isOk() const { return magic == extentSignature; }
         void assertOk() const { verify(isOk()); }
 
         Record* newRecord(int len);