Revert "SERVER-34385 Unit tests for secondary reads during oplog application"

This reverts commit e75fd5732ff4ecc269ab8a9cba601b79af460daa.

3 files changed, 5 insertions, 219 deletions

diff --git a/src/mongo/db/repl/sync_tail.cpp b/src/mongo/db/repl/sync_tail.cpp
index f26011121aa..4a455f00e77 100644
--- a/src/mongo/db/repl/sync_tail.cpp
+++ b/src/mongo/db/repl/sync_tail.cpp
@@ -92,8 +92,6 @@ AtomicInt32 SyncTail::replBatchLimitOperations{50 * 1000};
 
 namespace {
 
-MONGO_FP_DECLARE(pauseBatchApplicationBeforeCompletion);
-
 /**
  * This variable determines the number of writer threads SyncTail will have. It can be overridden
  * using the "replWriterThreadCount" server parameter.
@@ -1388,20 +1386,6 @@ StatusWith<OpTime> SyncTail::multiApply(OperationContext* opCtx, MultiApplier::O
         storageEngine->replicationBatchIsComplete();
     }
 
-    // Use this fail point to hold the PBWM lock and prevent the batch from completing.
-    if (MONGO_FAIL_POINT(pauseBatchApplicationBeforeCompletion)) {
-        log() << "pauseBatchApplicationBeforeCompletion fail point enabled. Blocking until fail "
-                 "point is disabled.";
-        while (MONGO_FAIL_POINT(pauseBatchApplicationBeforeCompletion)) {
-            if (inShutdown()) {
-                severe() << "Turn off pauseBatchApplicationBeforeCompletion before attempting "
-                            "clean shutdown";
-                fassertFailedNoTrace(50798);
-            }
-            sleepmillis(100);
-        }
-    }
-
     Timestamp firstTimeInBatch = ops.front().getTimestamp();
     // Set any indexes to multikey that this batch ignored. This must be done while holding the
     // parallel batch writer mutex.
diff --git a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
index 7fb1905c2b8..3f2329ae520 100644
--- a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
+++ b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
@@ -151,29 +151,18 @@ public:
         return itCountOn(op);
     }
 
-    int itCountLocal() {
+    boost::optional<Record> readRecordCommitted(RecordId id) {
         auto op = makeOperation();
         op->recoveryUnit()->setReadConcernLevelAndReplicationMode(
-            repl::ReadConcernLevel::kLocalReadConcern, repl::ReplicationCoordinator::modeReplSet);
-        op->recoveryUnit()->setShouldReadAtLastAppliedTimestamp(true);
-        return itCountOn(op);
-    }
-
-    boost::optional<Record> readRecordOn(OperationContext* op, RecordId id) {
+            repl::ReadConcernLevel::kMajorityReadConcern,
+            repl::ReplicationCoordinator::modeReplSet);
+        ASSERT_OK(op->recoveryUnit()->obtainMajorityCommittedSnapshot());
         auto cursor = rs->getCursor(op);
         auto record = cursor->seekExact(id);
         if (record)
             record->data.makeOwned();
         return record;
     }
-    boost::optional<Record> readRecordCommitted(RecordId id) {
-        auto op = makeOperation();
-        op->recoveryUnit()->setReadConcernLevelAndReplicationMode(
-            repl::ReadConcernLevel::kMajorityReadConcern,
-            repl::ReplicationCoordinator::modeReplSet);
-        ASSERT_OK(op->recoveryUnit()->obtainMajorityCommittedSnapshot());
-        return readRecordOn(op, id);
-    }
 
     std::string readStringCommitted(RecordId id) {
         auto record = readRecordCommitted(id);
@@ -181,20 +170,6 @@ public:
         return std::string(record->data.data());
     }
 
-    boost::optional<Record> readRecordLocal(RecordId id) {
-        auto op = makeOperation();
-        op->recoveryUnit()->setReadConcernLevelAndReplicationMode(
-            repl::ReadConcernLevel::kLocalReadConcern, repl::ReplicationCoordinator::modeReplSet);
-        op->recoveryUnit()->setShouldReadAtLastAppliedTimestamp(true);
-        return readRecordOn(op, id);
-    }
-
-    std::string readStringLocal(RecordId id) {
-        auto record = readRecordLocal(id);
-        ASSERT(record);
-        return std::string(record->data.data());
-    }
-
     void setUp() override {
         helper = KVHarnessHelper::create();
         engine = helper->getEngine();
@@ -371,76 +346,4 @@ TEST_F(SnapshotManagerTests, UpdateAndDelete) {
     ASSERT(!readRecordCommitted(id));
 }
 
-TEST_F(SnapshotManagerTests, InsertAndReadOnLocalSnapshot) {
-    if (!snapshotManager)
-        return;  // This test is only for engines that DO support SnapshotManagers.
-
-    auto beforeInsert = fetchAndIncrementTimestamp();
-
-    auto id = insertRecordAndCommit();
-    auto afterInsert = fetchAndIncrementTimestamp();
-
-    // Not reading on the last local timestamp returns the most recent data.
-    auto op = makeOperation();
-    auto ru = op->recoveryUnit();
-    ru->setShouldReadAtLastAppliedTimestamp(false);
-    ASSERT_EQ(itCountOn(op), 1);
-    ASSERT(readRecordOn(op, id));
-
-    deleteRecordAndCommit(id);
-    auto afterDelete = fetchAndIncrementTimestamp();
-
-    // Reading at the local snapshot timestamps returns data in order.
-    snapshotManager->setLocalSnapshot(beforeInsert);
-    ASSERT_EQ(itCountLocal(), 0);
-    ASSERT(!readRecordLocal(id));
-
-    snapshotManager->setLocalSnapshot(afterInsert);
-    ASSERT_EQ(itCountLocal(), 1);
-    ASSERT(readRecordLocal(id));
-
-    snapshotManager->setLocalSnapshot(afterDelete);
-    ASSERT_EQ(itCountLocal(), 0);
-    ASSERT(!readRecordLocal(id));
-}
-
-TEST_F(SnapshotManagerTests, UpdateAndDeleteOnLocalSnapshot) {
-    if (!snapshotManager)
-        return;  // This test is only for engines that DO support SnapshotManagers.
-
-    auto beforeInsert = fetchAndIncrementTimestamp();
-
-    auto id = insertRecordAndCommit("Aardvark");
-    auto afterInsert = fetchAndIncrementTimestamp();
-
-    updateRecordAndCommit(id, "Blue spotted stingray");
-    auto afterUpdate = fetchAndIncrementTimestamp();
-
-    // Not reading on the last local timestamp returns the most recent data.
-    auto op = makeOperation();
-    auto ru = op->recoveryUnit();
-    ru->setShouldReadAtLastAppliedTimestamp(false);
-    ASSERT_EQ(itCountOn(op), 1);
-    auto record = readRecordOn(op, id);
-    ASSERT_EQ(std::string(record->data.data()), "Blue spotted stingray");
-
-    deleteRecordAndCommit(id);
-    auto afterDelete = fetchAndIncrementTimestamp();
-
-    snapshotManager->setLocalSnapshot(beforeInsert);
-    ASSERT_EQ(itCountLocal(), 0);
-    ASSERT(!readRecordLocal(id));
-
-    snapshotManager->setLocalSnapshot(afterInsert);
-    ASSERT_EQ(itCountLocal(), 1);
-    ASSERT_EQ(readStringLocal(id), "Aardvark");
-
-    snapshotManager->setLocalSnapshot(afterUpdate);
-    ASSERT_EQ(itCountLocal(), 1);
-    ASSERT_EQ(readStringLocal(id), "Blue spotted stingray");
-
-    snapshotManager->setLocalSnapshot(afterDelete);
-    ASSERT_EQ(itCountLocal(), 0);
-    ASSERT(!readRecordLocal(id));
-}
 }  // namespace mongo
diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp
index d2c31fb6e1a..4426fe47a1b 100644
--- a/src/mongo/dbtests/storage_timestamp_tests.cpp
+++ b/src/mongo/dbtests/storage_timestamp_tests.cpp
@@ -50,9 +50,9 @@
 #include "mongo/db/op_observer_registry.h"
 #include "mongo/db/repl/apply_ops.h"
 #include "mongo/db/repl/drop_pending_collection_reaper.h"
-#include "mongo/db/repl/multiapplier.h"
 #include "mongo/db/repl/oplog.h"
 #include "mongo/db/repl/oplog_entry.h"
+#include "mongo/db/repl/oplog_entry.h"
 #include "mongo/db/repl/optime.h"
 #include "mongo/db/repl/repl_client_info.h"
 #include "mongo/db/repl/replication_consistency_markers_impl.h"
@@ -67,7 +67,6 @@
 #include "mongo/db/service_context.h"
 #include "mongo/db/storage/kv/kv_storage_engine.h"
 #include "mongo/dbtests/dbtests.h"
-#include "mongo/stdx/future.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/stacktrace.h"
 
@@ -1807,105 +1806,6 @@ public:
     }
 };
 
-class SecondaryReadsDuringBatchApplicationAreAllowed : public StorageTimestampTest {
-public:
-    void run() {
-        // Only run on 'wiredTiger'. No other storage engines to-date support timestamp writes.
-        if (mongo::storageGlobalParams.engine != "wiredTiger") {
-            return;
-        }
-        ASSERT(
-            _opCtx->getServiceContext()->getGlobalStorageEngine()->supportsReadConcernSnapshot());
-
-        NamespaceString ns("unittest.secondaryReadsDuringBatchApplicationAreAllowed");
-        reset(ns);
-        UUID uuid = UUID::gen();
-        {
-            AutoGetCollectionForRead autoColl(_opCtx, ns);
-            uuid = autoColl.getCollection()->uuid().get();
-            ASSERT_EQ(itCount(autoColl.getCollection()), 0);
-        }
-
-        // Returns true when the batch has started, meaning the applier is holding the PBWM lock.
-        // Will return false if the lock was not held.
-        Promise<bool> batchInProgressPromise;
-        // Attempt to read when in the middle of a batch.
-        stdx::packaged_task<bool()> task([&] {
-            Client::initThread(getThreadName());
-            auto readOp = cc().makeOperationContext();
-
-            // Wait for the batch to start or fail.
-            if (!batchInProgressPromise.getFuture().get()) {
-                return false;
-            }
-            AutoGetCollectionForRead autoColl(readOp.get(), ns);
-            return !readOp->lockState()->isLockHeldForMode(resourceIdParallelBatchWriterMode,
-                                                           MODE_IS);
-        });
-        auto taskFuture = task.get_future();
-        stdx::thread taskThread{std::move(task)};
-
-        auto joinGuard = MakeGuard([&] {
-            batchInProgressPromise.emplaceValue(false);
-            taskThread.join();
-        });
-
-        // This apply operation function will block until the reader has tried acquiring a
-        // collection lock. This returns BadValue statuses instead of asserting so that the worker
-        // threads can cleanly exit and this test case fails without crashing the entire suite.
-        auto applyOperationFn = [&](OperationContext* opCtx,
-                                    std::vector<const repl::OplogEntry*>* operationsToApply,
-                                    repl::SyncTail* st,
-                                    std::vector<MultikeyPathInfo>* pathInfo) -> Status {
-            if (!_opCtx->lockState()->isLockHeldForMode(resourceIdParallelBatchWriterMode,
-                                                        MODE_X)) {
-                return {ErrorCodes::BadValue, "Batch applied was not holding PBWM lock in MODE_X"};
-            }
-
-            // Insert the document. A reader without a PBWM lock should not see it yet.
-            auto status = repl::multiSyncApply(opCtx, operationsToApply, st, pathInfo);
-            if (!status.isOK()) {
-                return status;
-            }
-
-            // Signals the reader to acquire a collection read lock.
-            batchInProgressPromise.emplaceValue(true);
-
-            // Block while holding the PBWM lock until the reader is done.
-            if (!taskFuture.get()) {
-                return {ErrorCodes::BadValue, "Client was holding PBWM lock in MODE_IS"};
-            }
-            return Status::OK();
-        };
-
-        // Make a simple insert operation.
-        BSONObj doc0 = BSON("_id" << 0 << "a" << 0);
-        auto insertOp = repl::OplogEntry(
-            BSON("ts" << futureTs << "t" << 1LL << "h" << 0xBEEFBEEFLL << "v" << 2 << "op"
-                      << "i"
-                      << "ns"
-                      << ns.ns()
-                      << "ui"
-                      << uuid
-                      << "o"
-                      << doc0));
-
-        // Apply the operation.
-        auto writerPool = repl::SyncTail::makeWriterPool(1);
-        repl::SyncTail syncTail(nullptr, applyOperationFn, writerPool.get());
-        auto lastOpTime = unittest::assertGet(syncTail.multiApply(_opCtx, {insertOp}));
-        ASSERT_EQ(insertOp.getOpTime(), lastOpTime);
-
-        joinGuard.Dismiss();
-        taskThread.join();
-
-        // Read on the local snapshot to verify the document was inserted.
-        AutoGetCollectionForRead autoColl(_opCtx, ns);
-        assertDocumentAtTimestamp(autoColl.getCollection(), futureTs, doc0);
-    }
-};
-
-
 class AllStorageTimestampTests : public unittest::Suite {
 public:
     AllStorageTimestampTests() : unittest::Suite("StorageTimestampTests") {}
@@ -1933,7 +1833,6 @@ public:
         // TimestampIndexBuilds<SimulatePrimary>
         add<TimestampIndexBuilds<false>>();
         add<TimestampIndexBuilds<true>>();
-        add<SecondaryReadsDuringBatchApplicationAreAllowed>();
     }
 };