SERVER-22723 Remove unused noWarn flag and logIfError option

10 files changed, 43 insertions, 110 deletions

diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
index 6e5f5a5306b..6730670e60c 100644
--- a/src/mongo/db/catalog/collection.cpp
+++ b/src/mongo/db/catalog/collection.cpp
@@ -474,15 +474,12 @@ Status Collection::aboutToDeleteCapped(OperationContext* txn,
     _cursorManager.invalidateDocument(txn, loc, INVALIDATION_DELETION);
 
     BSONObj doc = data.releaseToBson();
-    _indexCatalog.unindexRecord(txn, doc, loc, false);
+    _indexCatalog.unindexRecord(txn, doc, loc);
 
     return Status::OK();
 }
 
-void Collection::deleteDocument(OperationContext* txn,
-                                const RecordId& loc,
-                                bool fromMigrate,
-                                bool noWarn) {
+void Collection::deleteDocument(OperationContext* txn, const RecordId& loc, bool fromMigrate) {
     if (isCapped()) {
         log() << "failing remove on a capped ns " << _ns << endl;
         uasserted(10089, "cannot remove from a capped collection");
@@ -497,7 +494,7 @@ void Collection::deleteDocument(OperationContext* txn,
     /* check if any cursors point to us.  if so, advance them. */
     _cursorManager.invalidateDocument(txn, loc, INVALIDATION_DELETION);
 
-    _indexCatalog.unindexRecord(txn, doc.value(), loc, noWarn);
+    _indexCatalog.unindexRecord(txn, doc.value(), loc);
 
     _recordStore->deleteRecord(txn, loc);
 
@@ -573,9 +570,7 @@ StatusWith<RecordId> Collection::updateDocument(OperationContext* txn,
             IndexCatalogEntry* entry = ii.catalogEntry(descriptor);
             IndexAccessMethod* iam = ii.accessMethod(descriptor);
 
-            InsertDeleteOptions options;
-            options.logIfError = false;
-            options.dupsAllowed =
+            bool dupsAllowed =
                 !(KeyPattern::isIdKeyPattern(descriptor->keyPattern()) || descriptor->unique()) ||
                 repl::getGlobalReplicationCoordinator()->shouldIgnoreUniqueIndex(descriptor);
             UpdateTicket* updateTicket = new UpdateTicket();
@@ -584,7 +579,7 @@ StatusWith<RecordId> Collection::updateDocument(OperationContext* txn,
                                              oldDoc.value(),
                                              newDoc,
                                              oldLocation,
-                                             options,
+                                             dupsAllowed,
                                              updateTicket,
                                              entry->getFilterExpression());
             if (!ret.isOK()) {
@@ -658,7 +653,7 @@ Status Collection::recordStoreGoingToMove(OperationContext* txn,
                                           size_t oldSize) {
     moveCounter.increment();
     _cursorManager.invalidateDocument(txn, oldLocation, INVALIDATION_DELETION);
-    _indexCatalog.unindexRecord(txn, BSONObj(oldBuffer), oldLocation, true);
+    _indexCatalog.unindexRecord(txn, BSONObj(oldBuffer), oldLocation);
     return Status::OK();
 }
 
diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index cb7417c4871..703501f8fa5 100644
--- a/src/mongo/db/catalog/collection.h
+++ b/src/mongo/db/catalog/collection.h
@@ -254,10 +254,7 @@ public:
      * 'noWarn' if unindexing the record causes an error, if noWarn is true the error
      * will not be logged.
      */
-    void deleteDocument(OperationContext* txn,
-                        const RecordId& loc,
-                        bool fromMigrate = false,
-                        bool noWarn = false);
+    void deleteDocument(OperationContext* txn, const RecordId& loc, bool fromMigrate = false);
 
     /*
      * Inserts all documents inside one WUOW.
diff --git a/src/mongo/db/catalog/index_catalog.cpp b/src/mongo/db/catalog/index_catalog.cpp
index 32139f520a8..bafd3c7835a 100644
--- a/src/mongo/db/catalog/index_catalog.cpp
+++ b/src/mongo/db/catalog/index_catalog.cpp
@@ -1112,15 +1112,13 @@ bool isDupsAllowed(IndexDescriptor* desc) {
 Status IndexCatalog::_indexFilteredRecords(OperationContext* txn,
                                            IndexCatalogEntry* index,
                                            const std::vector<BsonRecord>& bsonRecords) {
-    InsertDeleteOptions options;
-    options.logIfError = false;
-    options.dupsAllowed = isDupsAllowed(index->descriptor());
+    bool dupsAllowed = isDupsAllowed(index->descriptor());
 
     for (auto bsonRecord : bsonRecords) {
         int64_t inserted;
         invariant(bsonRecord.id != RecordId());
         Status status = index->accessMethod()->insert(
-            txn, *bsonRecord.docPtr, bsonRecord.id, options, &inserted);
+            txn, *bsonRecord.docPtr, bsonRecord.id, dupsAllowed, &inserted);
         if (!status.isOK())
             return status;
     }
@@ -1143,32 +1141,6 @@ Status IndexCatalog::_indexRecords(OperationContext* txn,
     return _indexFilteredRecords(txn, index, filteredBsonRecords);
 }
 
-Status IndexCatalog::_unindexRecord(OperationContext* txn,
-                                    IndexCatalogEntry* index,
-                                    const BSONObj& obj,
-                                    const RecordId& loc,
-                                    bool logIfError) {
-    InsertDeleteOptions options;
-    options.logIfError = logIfError;
-    options.dupsAllowed = isDupsAllowed(index->descriptor());
-
-    // For unindex operations, dupsAllowed=false really means that it is safe to delete anything
-    // that matches the key, without checking the RecordID, since dups are impossible. We need
-    // to disable this behavior for in-progress indexes. See SERVER-17487 for more details.
-    options.dupsAllowed = options.dupsAllowed || !index->isReady(txn);
-
-    int64_t removed;
-    Status status = index->accessMethod()->remove(txn, obj, loc, options, &removed);
-
-    if (!status.isOK()) {
-        log() << "Couldn't unindex record " << obj.toString() << " from collection "
-              << _collection->ns() << ". Status: " << status.toString();
-    }
-
-    return Status::OK();
-}
-
-
 Status IndexCatalog::indexRecords(OperationContext* txn,
                                   const std::vector<BsonRecord>& bsonRecords) {
     for (IndexCatalogEntryContainer::const_iterator i = _entries.begin(); i != _entries.end();
@@ -1181,17 +1153,19 @@ Status IndexCatalog::indexRecords(OperationContext* txn,
     return Status::OK();
 }
 
-void IndexCatalog::unindexRecord(OperationContext* txn,
-                                 const BSONObj& obj,
-                                 const RecordId& loc,
-                                 bool noWarn) {
-    for (IndexCatalogEntryContainer::const_iterator i = _entries.begin(); i != _entries.end();
-         ++i) {
-        IndexCatalogEntry* entry = *i;
+void IndexCatalog::unindexRecord(OperationContext* txn, const BSONObj& obj, const RecordId& loc) {
+    for (auto index : _entries) {
+        // For unindex operations, dupsAllowed=false really means that it is safe to delete anything
+        // that matches the key, without checking the RecordID, since dups are impossible. We need
+        // to disable this behavior for in-progress indexes. See SERVER-17487 for more details.
+        bool dupsAllowed = isDupsAllowed(index->descriptor()) || !index->isReady(txn);
 
-        // If it's a background index, we DO NOT want to log anything.
-        bool logIfError = entry->isReady(txn) ? !noWarn : false;
-        _unindexRecord(txn, entry, obj, loc, logIfError);
+        int64_t removed;
+        Status status = index->accessMethod()->remove(txn, obj, loc, dupsAllowed, &removed);
+
+        if (!status.isOK())
+            log() << "Couldn't unindex record " << obj.toString() << " from collection "
+                  << _collection->ns() << ". Status: " << status.toString();
     }
 }
 
diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h
index bb3edea07b1..bba5f523c2b 100644
--- a/src/mongo/db/catalog/index_catalog.h
+++ b/src/mongo/db/catalog/index_catalog.h
@@ -266,7 +266,7 @@ public:
     // this throws for now
     Status indexRecords(OperationContext* txn, const std::vector<BsonRecord>& bsonRecords);
 
-    void unindexRecord(OperationContext* txn, const BSONObj& obj, const RecordId& loc, bool noWarn);
+    void unindexRecord(OperationContext* txn, const BSONObj& obj, const RecordId& loc);
 
     // ------- temp internal -------
 
@@ -303,12 +303,6 @@ private:
                          IndexCatalogEntry* index,
                          const std::vector<BsonRecord>& bsonRecords);
 
-    Status _unindexRecord(OperationContext* txn,
-                          IndexCatalogEntry* index,
-                          const BSONObj& obj,
-                          const RecordId& loc,
-                          bool logIfError);
-
     /**
      * this does no sanity checks
      */
diff --git a/src/mongo/db/catalog/index_create.cpp b/src/mongo/db/catalog/index_create.cpp
index 1b2b3068530..534a5aab1df 100644
--- a/src/mongo/db/catalog/index_create.cpp
+++ b/src/mongo/db/catalog/index_create.cpp
@@ -193,8 +193,7 @@ Status MultiIndexBlock::init(const std::vector<BSONObj>& indexSpecs) {
 
         const IndexDescriptor* descriptor = index.block->getEntry()->descriptor();
 
-        index.options.logIfError = false;  // logging happens elsewhere if needed.
-        index.options.dupsAllowed = !descriptor->unique() || _ignoreUnique ||
+        index.dupsAllowed = !descriptor->unique() || _ignoreUnique ||
             repl::getGlobalReplicationCoordinator()->shouldIgnoreUniqueIndex(descriptor);
 
         log() << "build index on: " << ns << " properties: " << descriptor->toString();
@@ -313,9 +312,9 @@ Status MultiIndexBlock::insert(const BSONObj& doc, const RecordId& loc) {
         int64_t unused;
         Status idxStatus(ErrorCodes::InternalError, "");
         if (_indexes[i].bulk) {
-            idxStatus = _indexes[i].bulk->insert(_txn, doc, loc, _indexes[i].options, &unused);
+            idxStatus = _indexes[i].bulk->insert(_txn, doc, loc);
         } else {
-            idxStatus = _indexes[i].real->insert(_txn, doc, loc, _indexes[i].options, &unused);
+            idxStatus = _indexes[i].real->insert(_txn, doc, loc, _indexes[i].dupsAllowed, &unused);
         }
 
         if (!idxStatus.isOK())
@@ -333,7 +332,7 @@ Status MultiIndexBlock::doneInserting(std::set<RecordId>* dupsOut) {
         Status status = _indexes[i].real->commitBulk(_txn,
                                                      std::move(_indexes[i].bulk),
                                                      _allowInterruption,
-                                                     _indexes[i].options.dupsAllowed,
+                                                     _indexes[i].dupsAllowed,
                                                      dupsOut);
         if (!status.isOK()) {
             return status;
diff --git a/src/mongo/db/catalog/index_create.h b/src/mongo/db/catalog/index_create.h
index d3dccb20b46..80694a50ea4 100644
--- a/src/mongo/db/catalog/index_create.h
+++ b/src/mongo/db/catalog/index_create.h
@@ -217,7 +217,7 @@ private:
         const MatchExpression* filterExpression;  // might be NULL, owned elsewhere
         std::unique_ptr<IndexAccessMethod::BulkBuilder> bulk;
 
-        InsertDeleteOptions options;
+        bool dupsAllowed = false;
     };
 
     std::vector<IndexToBuild> _indexes;
diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp
index cce155af8cd..5633d19fe11 100644
--- a/src/mongo/db/cloner.cpp
+++ b/src/mongo/db/cloner.cpp
@@ -611,7 +611,7 @@ Status Cloner::copyDb(OperationContext* txn,
                 // dupsAllowed in IndexCatalog::_unindexRecord and SERVER-17487.
                 for (set<RecordId>::const_iterator it = dups.begin(); it != dups.end(); ++it) {
                     WriteUnitOfWork wunit(txn);
-                    c->deleteDocument(txn, *it, false, true);
+                    c->deleteDocument(txn, *it, false);
                     wunit.commit();
                 }
 
diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index f99d3104c30..2bb7ee67cb4 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -100,7 +100,7 @@ bool IndexAccessMethod::ignoreKeyTooLong(OperationContext* txn) {
 Status IndexAccessMethod::insert(OperationContext* txn,
                                  const BSONObj& obj,
                                  const RecordId& loc,
-                                 const InsertDeleteOptions& options,
+                                 bool dupsAllowed,
                                  int64_t* numInserted) {
     *numInserted = 0;
 
@@ -110,7 +110,7 @@ Status IndexAccessMethod::insert(OperationContext* txn,
 
     Status ret = Status::OK();
     for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) {
-        Status status = _newInterface->insert(txn, *i, loc, options.dupsAllowed);
+        Status status = _newInterface->insert(txn, *i, loc, dupsAllowed);
 
         // Everything's OK, carry on.
         if (status.isOK()) {
@@ -135,7 +135,7 @@ Status IndexAccessMethod::insert(OperationContext* txn,
 
         // Clean up after ourselves.
         for (BSONObjSet::const_iterator j = keys.begin(); j != i; ++j) {
-            removeOneKey(txn, *j, loc, options.dupsAllowed);
+            removeOneKey(txn, *j, loc, dupsAllowed);
             *numInserted = 0;
         }
 
@@ -177,14 +177,14 @@ std::unique_ptr<SortedDataInterface::Cursor> IndexAccessMethod::newRandomCursor(
 Status IndexAccessMethod::remove(OperationContext* txn,
                                  const BSONObj& obj,
                                  const RecordId& loc,
-                                 const InsertDeleteOptions& options,
+                                 bool dupsAllowed,
                                  int64_t* numDeleted) {
     BSONObjSet keys;
     getKeys(obj, &keys);
     *numDeleted = 0;
 
     for (BSONObjSet::const_iterator i = keys.begin(); i != keys.end(); ++i) {
-        removeOneKey(txn, *i, loc, options.dupsAllowed);
+        removeOneKey(txn, *i, loc, dupsAllowed);
         ++*numDeleted;
     }
 
@@ -271,7 +271,7 @@ Status IndexAccessMethod::validateUpdate(OperationContext* txn,
                                          const BSONObj& from,
                                          const BSONObj& to,
                                          const RecordId& record,
-                                         const InsertDeleteOptions& options,
+                                         bool dupsAllowed,
                                          UpdateTicket* ticket,
                                          const MatchExpression* indexFilter) {
     if (indexFilter == NULL || indexFilter->matchesBSON(from))
@@ -279,7 +279,7 @@ Status IndexAccessMethod::validateUpdate(OperationContext* txn,
     if (indexFilter == NULL || indexFilter->matchesBSON(to))
         getKeys(to, &ticket->newKeys);
     ticket->loc = record;
-    ticket->dupsAllowed = options.dupsAllowed;
+    ticket->dupsAllowed = dupsAllowed;
 
     setDifference(ticket->oldKeys, ticket->newKeys, &ticket->removed);
     setDifference(ticket->newKeys, ticket->oldKeys, &ticket->added);
@@ -342,9 +342,7 @@ IndexAccessMethod::BulkBuilder::BulkBuilder(const IndexAccessMethod* index,
 
 Status IndexAccessMethod::BulkBuilder::insert(OperationContext* txn,
                                               const BSONObj& obj,
-                                              const RecordId& loc,
-                                              const InsertDeleteOptions& options,
-                                              int64_t* numInserted) {
+                                              const RecordId& loc) {
     BSONObjSet keys;
     _real->getKeys(obj, &keys);
 
@@ -355,10 +353,6 @@ Status IndexAccessMethod::BulkBuilder::insert(OperationContext* txn,
         _keysInserted++;
     }
 
-    if (NULL != numInserted) {
-        *numInserted += keys.size();
-    }
-
     return Status::OK();
 }
 
diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h
index f0376af716a..3aafc1f07d2 100644
--- a/src/mongo/db/index/index_access_method.h
+++ b/src/mongo/db/index/index_access_method.h
@@ -46,7 +46,6 @@ extern std::atomic<bool> failIndexKeyTooLong;  // NOLINT
 class BSONObjBuilder;
 class MatchExpression;
 class UpdateTicket;
-struct InsertDeleteOptions;
 
 /**
  * An IndexAccessMethod is the interface through which all the mutation, lookup, and
@@ -74,23 +73,22 @@ public:
      * 'loc') into the index.  'obj' is the object at the location 'loc'.  If not NULL,
      * 'numInserted' will be set to the number of keys added to the index for the document.  If
      * there is more than one key for 'obj', either all keys will be inserted or none will.
-     *
-     * The behavior of the insertion can be specified through 'options'.
      */
     Status insert(OperationContext* txn,
                   const BSONObj& obj,
                   const RecordId& loc,
-                  const InsertDeleteOptions& options,
+                  bool dupsAllowed,
                   int64_t* numInserted);
 
     /**
      * Analogous to above, but remove the records instead of inserting them.  If not NULL,
      * numDeleted will be set to the number of keys removed from the index for the document.
+     * If dupsAllowed is false, the record can be unindexed without checking its loc.
      */
     Status remove(OperationContext* txn,
                   const BSONObj& obj,
                   const RecordId& loc,
-                  const InsertDeleteOptions& options,
+                  bool dupsAllowed,
                   int64_t* numDeleted);
 
     /**
@@ -107,7 +105,7 @@ public:
                           const BSONObj& from,
                           const BSONObj& to,
                           const RecordId& loc,
-                          const InsertDeleteOptions& options,
+                          bool dupsAllowed,
                           UpdateTicket* ticket,
                           const MatchExpression* indexFilter);
 
@@ -203,11 +201,7 @@ public:
         /**
          * Insert into the BulkBuilder as-if inserting into an IndexAccessMethod.
          */
-        Status insert(OperationContext* txn,
-                      const BSONObj& obj,
-                      const RecordId& loc,
-                      const InsertDeleteOptions& options,
-                      int64_t* numInserted);
+        Status insert(OperationContext* txn, const BSONObj& obj, const RecordId& loc);
 
     private:
         friend class IndexAccessMethod;
@@ -288,18 +282,4 @@ private:
     RecordId loc;
     bool dupsAllowed;
 };
-
-/**
- * Flags we can set for inserts and deletes (and updates, which are kind of both).
- */
-struct InsertDeleteOptions {
-    InsertDeleteOptions() : logIfError(false), dupsAllowed(false) {}
-
-    // If there's an error, log() it.
-    bool logIfError;
-
-    // Are duplicate keys allowed in the index?
-    bool dupsAllowed;
-};
-
 }  // namespace mongo
diff --git a/src/mongo/db/storage/sorted_data_interface.h b/src/mongo/db/storage/sorted_data_interface.h
index eeefb10d5ea..410cf64c043 100644
--- a/src/mongo/db/storage/sorted_data_interface.h
+++ b/src/mongo/db/storage/sorted_data_interface.h
@@ -104,8 +104,8 @@ public:
      * Remove the entry from the index with the specified key and RecordId.
      *
      * @param txn the transaction under which the remove takes place
-     * @param dupsAllowed true if duplicate keys are allowed, and false
-     *        otherwise
+     * @param dupsAllowed true if duplicate keys are allowed. If false, no duplicate keys exist
+     *        and it is not necessary to check the RecordId.
      */
     virtual void unindex(OperationContext* txn,
                          const BSONObj& key,