From 906ac3ca78d352df2d0dd45350195251efe0dea1 Mon Sep 17 00:00:00 2001 From: Louis Williams Date: Wed, 9 Oct 2019 19:56:36 +0000 Subject: [PATCH] SERVER-43638 Do not block prepared transactions on index builds on secondaries --- .../index_builds_ignore_prepare_conflicts.js | 1 - .../db/repl/transaction_oplog_application.cpp | 26 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/jstests/noPassthrough/index_builds_ignore_prepare_conflicts.js b/jstests/noPassthrough/index_builds_ignore_prepare_conflicts.js index a26736e3ebf..08751e09728 100644 --- a/jstests/noPassthrough/index_builds_ignore_prepare_conflicts.js +++ b/jstests/noPassthrough/index_builds_ignore_prepare_conflicts.js @@ -5,7 +5,6 @@ * @tags: [ * requires_document_locking, * requires_replication, - * two_phase_index_builds_unsupported, * uses_prepare_transaction, * uses_transactions, * ] diff --git a/src/mongo/db/repl/transaction_oplog_application.cpp b/src/mongo/db/repl/transaction_oplog_application.cpp index 1c39ddb79b3..4d887c90e27 100644 --- a/src/mongo/db/repl/transaction_oplog_application.cpp +++ b/src/mongo/db/repl/transaction_oplog_application.cpp @@ -324,14 +324,24 @@ Status _applyPrepareTransaction(OperationContext* opCtx, // This will prevent hybrid index builds from corrupting an index on secondary nodes if a // prepared transaction becomes prepared during a build but commits after the index build // commits. - for (const auto& op : ops) { - auto ns = op.getNss(); - auto uuid = *op.getUuid(); - if (BackgroundOperation::inProgForNs(ns)) { - warning() << "blocking replication until index builds are finished on " - << redact(ns.toString()) << ", due to prepared transaction"; - BackgroundOperation::awaitNoBgOpInProgForNs(ns); - IndexBuildsCoordinator::get(opCtx)->awaitNoIndexBuildInProgressForCollection(uuid); + // When two-phase index builds are in use, this is both unnecessary and unsafe. Due to locking, + // we can guarantee that a transaction prepared on a primary during an index build will always + // commit before that index build completes. Because two-phase index builds replicate start and + // commit oplog entries, it will never be possible to replicate a prepared transaction, commit + // an index build, then commit the transaction, the bug described above. + // This blocking behavior can also introduce a deadlock with two-phase index builds on + // a secondary if a prepared transaction blocks on an index build, but the index build can't + // re-acquire its X lock because of the transaction. + if (!IndexBuildsCoordinator::get(opCtx)->supportsTwoPhaseIndexBuild()) { + for (const auto& op : ops) { + auto ns = op.getNss(); + auto uuid = *op.getUuid(); + if (BackgroundOperation::inProgForNs(ns)) { + warning() << "blocking replication until index builds are finished on " + << redact(ns.toString()) << ", due to prepared transaction"; + BackgroundOperation::awaitNoBgOpInProgForNs(ns); + IndexBuildsCoordinator::get(opCtx)->awaitNoIndexBuildInProgressForCollection(uuid); + } } }