SERVER-42563 Rename and remove try...catch around rollback critical section

3 files changed, 12 insertions, 31 deletions

diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp
index 73c484ec452..5ae71455c1d 100644
--- a/src/mongo/db/repl/rollback_impl.cpp
+++ b/src/mongo/db/repl/rollback_impl.cpp
@@ -233,13 +233,8 @@ Status RollbackImpl::runRollback(OperationContext* opCtx) {
     _rollbackStats.rollbackId = _replicationProcess->getRollbackID();
     _listener->onRollbackIDIncremented();
 
-    // Execute the critical section in rollback. It is illegal to exit rollback cleanly between
-    // aborting prepared transactions and reconstructing them. During this window, no interruptions
-    // are allowed and all errors should be made fatal.
-    status = _runRollbackCriticalSection(opCtx, commonPoint);
-    if (!status.isOK()) {
-        fassertFailedWithStatus(31049, status.withContext("Error in rollback critical section"));
-    }
+    // This function cannot fail without terminating the process.
+    _runPhaseFromAbortToReconstructPreparedTxns(opCtx, commonPoint);
     _listener->onPreparedTransactionsReconstructed();
 
     // We can now accept interruptions again.
@@ -451,9 +446,8 @@ StatusWith<std::set<NamespaceString>> RollbackImpl::_namespacesForOp(const Oplog
     return namespaces;
 }
 
-Status RollbackImpl::_runRollbackCriticalSection(
-    OperationContext* opCtx,
-    RollBackLocalOperations::RollbackCommonPoint commonPoint) noexcept try {
+void RollbackImpl::_runPhaseFromAbortToReconstructPreparedTxns(
+    OperationContext* opCtx, RollBackLocalOperations::RollbackCommonPoint commonPoint) noexcept {
     // Before computing record store counts, abort all active transactions. This ensures that
     // the count adjustments are based on correct values where no prepared transactions are
     // active and all in-memory counts have been rolled-back.
@@ -468,9 +462,7 @@ Status RollbackImpl::_runRollbackCriticalSection(
     // rollback. Note: these numbers are relative to the common point, not the stable timestamp,
     // and thus must be set after recovering from the oplog.
     auto status = _findRecordStoreCounts(opCtx);
-    if (!status.isOK()) {
-        return status.withContext("Error while finding record store counts");
-    }
+    fassert(31227, status);
 
     if (shouldCreateDataFiles()) {
         // Write a rollback file for each namespace that has documents that would be deleted by
@@ -478,9 +470,7 @@ Status RollbackImpl::_runRollbackCriticalSection(
         // unecessary prepare conflicts when trying to read documents that were modified by
         // those prepared transactions, which we know we will abort anyway.
         status = _writeRollbackFiles(opCtx);
-        if (!status.isOK()) {
-            return status.withContext("Error while writing out rollback files");
-        }
+        fassert(31228, status);
     } else {
         log() << "Not writing rollback files. 'createRollbackDataFiles' set to false.";
     }
@@ -494,10 +484,8 @@ Status RollbackImpl::_runRollbackCriticalSection(
 
     // Recover to the stable timestamp.
     auto stableTimestampSW = _recoverToStableTimestamp(opCtx);
-    if (!stableTimestampSW.isOK()) {
-        auto status = stableTimestampSW.getStatus();
-        return status.withContext("Error while recovering to stable timestamp");
-    }
+    fassert(31049, stableTimestampSW);
+
     _rollbackStats.stableTimestamp = stableTimestampSW.getValue();
     _listener->onRecoverToStableTimestamp(stableTimestampSW.getValue());
 
@@ -547,12 +535,6 @@ Status RollbackImpl::_runRollbackCriticalSection(
     // collection counts, reconstruct the prepared transactions now, adding on any additional counts
     // to the now corrected record store.
     reconstructPreparedTransactions(opCtx, OplogApplication::Mode::kRecovering);
-
-    return Status::OK();
-} catch (...) {
-    // Any exceptions here should be made fatal.
-    severe() << "Caught exception during critical section in rollback: " << exceptionToStatus();
-    std::terminate();
 }
 
 void RollbackImpl::_correctRecordStoreCounts(OperationContext* opCtx) {
diff --git a/src/mongo/db/repl/rollback_impl.h b/src/mongo/db/repl/rollback_impl.h
index 00ca82d3fef..5b32d6abb32 100644
--- a/src/mongo/db/repl/rollback_impl.h
+++ b/src/mongo/db/repl/rollback_impl.h
@@ -393,10 +393,10 @@ private:
     Status _findRecordStoreCounts(OperationContext* opCtx);
 
     /**
-     * Executes the critical section in rollback, defined as the window between aborting and
-     * reconstructing prepared transactions.
+     * Executes the phase of rollback between aborting and reconstructing prepared transactions. We
+     * cannot safely recover if we fail during this phase.
      */
-    Status _runRollbackCriticalSection(
+    void _runPhaseFromAbortToReconstructPreparedTxns(
         OperationContext* opCtx, RollBackLocalOperations::RollbackCommonPoint commonPoint) noexcept;
 
     /**
diff --git a/src/mongo/db/repl/rollback_impl_test.cpp b/src/mongo/db/repl/rollback_impl_test.cpp
index 1f514eda2e6..05ce9e82cf9 100644
--- a/src/mongo/db/repl/rollback_impl_test.cpp
+++ b/src/mongo/db/repl/rollback_impl_test.cpp
@@ -766,7 +766,7 @@ TEST_F(RollbackImplTest,
 
 DEATH_TEST_F(RollbackImplTest,
              RollbackUassertsAreFatalBetweenAbortingAndReconstructingPreparedTransactions,
-             "Caught exception during critical section in rollback") {
+             "UnknownError: error for test") {
     auto op = makeOpAndRecordId(1);
     _remoteOplog->setOperations({op});
     ASSERT_OK(_insertOplogEntry(op.first));
@@ -779,7 +779,6 @@ DEATH_TEST_F(RollbackImplTest,
     // Called before aborting prepared transactions.
     _onRollbackIDIncrementedFn = [this]() { _incrementedRollbackID = true; };
 
-    // Called during the critical section.
     _onRecoverToStableTimestampFn = [this](Timestamp stableTimestamp) {
         _recoveredToStableTimestamp = true;
         uasserted(ErrorCodes::UnknownError, "error for test");