SERVER-15700 Modify SortedDataInterface to not require unindex() to return

whether the index row was deleted. Fix unit tests to manually verify that
unindex() operations had the desired effect (in the same operation context)

Closes #827

Signed-off-by: Benety Goh <benety@mongodb.com>

9 files changed, 43 insertions, 38 deletions

diff --git a/src/mongo/db/index/btree_based_access_method.cpp b/src/mongo/db/index/btree_based_access_method.cpp
index 12fac43e15e..b4e1ba833b0 100644
--- a/src/mongo/db/index/btree_based_access_method.cpp
+++ b/src/mongo/db/index/btree_based_access_method.cpp
@@ -124,14 +124,12 @@ namespace mongo {
         return ret;
     }
 
-    bool BtreeBasedAccessMethod::removeOneKey(OperationContext* txn,
+    void BtreeBasedAccessMethod::removeOneKey(OperationContext* txn,
                                               const BSONObj& key,
                                               const DiskLoc& loc,
                                               bool dupsAllowed) {
-        bool ret = false;
-
         try {
-            ret = _newInterface->unindex(txn, key, loc, dupsAllowed);
+            _newInterface->unindex(txn, key, loc, dupsAllowed);
         } catch (AssertionException& e) {
             log() << "Assertion failure: _unindex failed "
                   << _descriptor->indexNamespace() << endl;
@@ -140,8 +138,6 @@ namespace mongo {
             log() << "  dl:" << loc.toString() << endl;
             logContext();
         }
-
-        return ret;
     }
 
     Status BtreeBasedAccessMethod::newCursor(OperationContext* txn, const CursorOptions& opts, IndexCursor** out) const {
@@ -161,14 +157,8 @@ namespace mongo {
         *numDeleted = 0;
 
         for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) {
-            bool thisKeyOK = removeOneKey(txn, *i, loc, options.dupsAllowed);
-
-            if (thisKeyOK) {
-                ++*numDeleted;
-            } else if (options.logIfError) {
-                log() << "unindex failed (key too big?) " << _descriptor->indexNamespace()
-                      << " key: " << *i;
-            }
+            removeOneKey(txn, *i, loc, options.dupsAllowed);
+            ++*numDeleted;
         }
 
         return Status::OK();
diff --git a/src/mongo/db/index/btree_based_access_method.h b/src/mongo/db/index/btree_based_access_method.h
index f30a2e946de..3a5dfee4630 100644
--- a/src/mongo/db/index/btree_based_access_method.h
+++ b/src/mongo/db/index/btree_based_access_method.h
@@ -132,7 +132,7 @@ namespace mongo {
         const IndexDescriptor* _descriptor;
 
     private:
-        bool removeOneKey(OperationContext* txn,
+        void removeOneKey(OperationContext* txn,
                           const BSONObj& key,
                           const DiskLoc& loc,
                           bool dupsAllowed);
diff --git a/src/mongo/db/storage/heap1/heap1_btree_impl.cpp b/src/mongo/db/storage/heap1/heap1_btree_impl.cpp
index f8f93f3205c..674c9f7a0ac 100644
--- a/src/mongo/db/storage/heap1/heap1_btree_impl.cpp
+++ b/src/mongo/db/storage/heap1/heap1_btree_impl.cpp
@@ -170,7 +170,7 @@ namespace {
             return Status::OK();
         }
 
-        virtual bool unindex(OperationContext* txn,
+        virtual void unindex(OperationContext* txn,
                              const BSONObj& key,
                              const DiskLoc& loc,
                              bool dupsAllowed) {
@@ -184,8 +184,6 @@ namespace {
                 _currentKeySize -= key.objsize();
                 Heap1RecoveryUnit::notifyIndexRemove( txn, this, key, loc );
             }
-
-            return numDeleted == 1;
         }
 
         virtual void fullValidate(OperationContext* txn, long long *numKeysOut) const {
diff --git a/src/mongo/db/storage/mmap_v1/btree/btree_interface.cpp b/src/mongo/db/storage/mmap_v1/btree/btree_interface.cpp
index 6ddb358edf3..90b97d2de58 100644
--- a/src/mongo/db/storage/mmap_v1/btree/btree_interface.cpp
+++ b/src/mongo/db/storage/mmap_v1/btree/btree_interface.cpp
@@ -85,12 +85,12 @@ namespace mongo {
             return _btree->insert(txn, key, loc, dupsAllowed);
         }
 
-        virtual bool unindex(OperationContext* txn,
+        virtual void unindex(OperationContext* txn,
                              const BSONObj& key,
                              const DiskLoc& loc,
                              bool dupsAllowed) {
 
-            return _btree->unindex(txn, key, loc);
+            _btree->unindex(txn, key, loc);
         }
 
         virtual void fullValidate(OperationContext* txn, long long *numKeysOut) const {
diff --git a/src/mongo/db/storage/sorted_data_interface.h b/src/mongo/db/storage/sorted_data_interface.h
index a7888acad75..34b9cca2a2b 100644
--- a/src/mongo/db/storage/sorted_data_interface.h
+++ b/src/mongo/db/storage/sorted_data_interface.h
@@ -77,7 +77,7 @@ namespace mongo {
                               const DiskLoc& loc,
                               bool dupsAllowed) = 0;
 
-        virtual bool unindex(OperationContext* txn,
+        virtual void unindex(OperationContext* txn,
                              const BSONObj& key,
                              const DiskLoc& loc,
                              bool dupsAllowed) = 0;
diff --git a/src/mongo/db/storage/sorted_data_interface_test_harness.cpp b/src/mongo/db/storage/sorted_data_interface_test_harness.cpp
index b731931de92..dd04bb3945f 100644
--- a/src/mongo/db/storage/sorted_data_interface_test_harness.cpp
+++ b/src/mongo/db/storage/sorted_data_interface_test_harness.cpp
@@ -203,7 +203,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( !sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 20 ), true ) );
+                sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 20 ), true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -217,7 +218,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( !sorted->unindex( opCtx.get(), BSON( "" << 2 ), DiskLoc( 5, 18 ), true ) );
+                sorted->unindex( opCtx.get(), BSON( "" << 2 ), DiskLoc( 5, 18 ), true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -232,7 +234,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 18 ), true ) );
+                sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 18 ), true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -266,7 +269,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 18 ), true ) );
+                sorted->unindex( opCtx.get(), BSON( "" << 1 ), DiskLoc( 5, 18 ), true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 // no commit
             }
         }
diff --git a/src/mongo/db/storage/sorted_data_interface_test_isempty.cpp b/src/mongo/db/storage/sorted_data_interface_test_isempty.cpp
index 11605c39e1e..1a85e25b80d 100644
--- a/src/mongo/db/storage/sorted_data_interface_test_isempty.cpp
+++ b/src/mongo/db/storage/sorted_data_interface_test_isempty.cpp
@@ -65,7 +65,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc1, false ) );
+                sorted->unindex( opCtx.get(), key1, loc1, false );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
diff --git a/src/mongo/db/storage/sorted_data_interface_test_rollback.cpp b/src/mongo/db/storage/sorted_data_interface_test_rollback.cpp
index 0216f35f575..dc6da2a988e 100644
--- a/src/mongo/db/storage/sorted_data_interface_test_rollback.cpp
+++ b/src/mongo/db/storage/sorted_data_interface_test_rollback.cpp
@@ -107,7 +107,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key2, loc2, true ) );
+                sorted->unindex( opCtx.get(), key2, loc2, true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 // no commit
             }
         }
@@ -135,8 +136,10 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc1, true ) );
-                ASSERT( sorted->unindex( opCtx.get(), key3, loc3, true ) );
+                sorted->unindex( opCtx.get(), key1, loc1, true );
+                ASSERT_EQUALS( 2, sorted->numEntries( opCtx.get() ) );
+                sorted->unindex( opCtx.get(), key3, loc3, true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 // no commit
             }
         }
diff --git a/src/mongo/db/storage/sorted_data_interface_test_unindex.cpp b/src/mongo/db/storage/sorted_data_interface_test_unindex.cpp
index 4aeb8174e48..1ca69411417 100644
--- a/src/mongo/db/storage/sorted_data_interface_test_unindex.cpp
+++ b/src/mongo/db/storage/sorted_data_interface_test_unindex.cpp
@@ -63,7 +63,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc1, true ) );
+                sorted->unindex( opCtx.get(), key1, loc1, true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -102,7 +103,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), compoundKey1a, loc1, true ) );
+                sorted->unindex( opCtx.get(), compoundKey1a, loc1, true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -142,7 +144,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key2, loc2, true ) );
+                sorted->unindex( opCtx.get(), key2, loc2, true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -170,8 +173,10 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc1, true ) );
-                ASSERT( sorted->unindex( opCtx.get(), key3, loc3, true ) );
+                sorted->unindex( opCtx.get(), key1, loc1, true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
+                sorted->unindex( opCtx.get(), key3, loc3, true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -211,7 +216,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc2, true ) );
+                sorted->unindex( opCtx.get(), key1, loc2, true );
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -239,8 +245,10 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc1, true) );
-                ASSERT( sorted->unindex( opCtx.get(), key1, loc3, true ) );
+                sorted->unindex( opCtx.get(), key1, loc1, true);
+                ASSERT_EQUALS( 1, sorted->numEntries( opCtx.get() ) );
+                sorted->unindex( opCtx.get(), key1, loc3, true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }
@@ -265,7 +273,8 @@ namespace mongo {
             scoped_ptr<OperationContext> opCtx( harnessHelper->newOperationContext() );
             {
                 WriteUnitOfWork uow( opCtx.get() );
-                ASSERT( !sorted->unindex( opCtx.get(), key1, loc1, true ) );
+                sorted->unindex( opCtx.get(), key1, loc1, true );
+                ASSERT( sorted->isEmpty( opCtx.get() ) );
                 uow.commit();
             }
         }