From 700429506560c319cd6c106f127366ddbb8f6d85 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 4 Feb 2025 11:10:37 +0300 Subject: [PATCH] Revert "Release RowExclusiveLock on pg_dist_transaction as soon as remote xacts are recovered" This reverts commit 684b4c6b9666954bdf13c9eb597390eb71a66d48. --- .../transaction/transaction_recovery.c | 18 +----------------- .../isolation_create_restore_point.out | 7 ++++--- .../spec/isolation_create_restore_point.spec | 5 +---- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/src/backend/distributed/transaction/transaction_recovery.c b/src/backend/distributed/transaction/transaction_recovery.c index 7cf684d56..0eede84ca 100644 --- a/src/backend/distributed/transaction/transaction_recovery.c +++ b/src/backend/distributed/transaction/transaction_recovery.c @@ -416,23 +416,7 @@ RecoverWorkerTransactions(WorkerNode *workerNode) } systable_endscan(scanDescriptor); - - /* - * Here we release the lock on pg_dist_transaction while closing it to avoid - * deadlocks that might occur because of trying to acquire a lock on - * pg_dist_authinfo while holding a lock on pg_dist_transaction. Such a scenario - * can only cause a deadlock if another transaction is trying to acquire a strong - * lock on pg_dist_transaction while holding a lock on pg_dist_authinfo. As of - * today, we (implicitly) acquire a strong lock on pg_dist_transaction only when - * upgrading Citus to 11.3-1 and this happens when creating a REPLICA IDENTITY on - * pg_dist_transaction. - * - * And reglardless of the code-path we are in, it should be okay to release the - * lock now because all we do after this point is to abort the prepared - * transactions that are not part of an in-progress distributed transaction and - * releasing the lock before doing so should be just fine. - */ - table_close(pgDistTransaction, RowExclusiveLock); + table_close(pgDistTransaction, NoLock); if (!recoveryFailed) { diff --git a/src/test/regress/expected/isolation_create_restore_point.out b/src/test/regress/expected/isolation_create_restore_point.out index dce15a35d..3b1bdf9eb 100644 --- a/src/test/regress/expected/isolation_create_restore_point.out +++ b/src/test/regress/expected/isolation_create_restore_point.out @@ -147,15 +147,16 @@ recover_prepared_transactions step s2-create-restore: SELECT 1 FROM citus_create_restore_point('citus-test'); + +step s1-commit: + COMMIT; +step s2-create-restore: <... completed> ?column? --------------------------------------------------------------------- 1 (1 row) -step s1-commit: - COMMIT; - starting permutation: s1-begin s1-drop s2-create-restore s1-commit create_reference_table diff --git a/src/test/regress/spec/isolation_create_restore_point.spec b/src/test/regress/spec/isolation_create_restore_point.spec index c62a64a44..2cdc66f85 100644 --- a/src/test/regress/spec/isolation_create_restore_point.spec +++ b/src/test/regress/spec/isolation_create_restore_point.spec @@ -154,10 +154,7 @@ permutation "s1-begin" "s1-ddl" "s2-create-restore" "s1-commit" // verify that citus_create_restore_point is not blocked by concurrent COPY (only commit) permutation "s1-begin" "s1-copy" "s2-create-restore" "s1-commit" -// verify that citus_create_restore_point is partially blocked by concurrent recover_prepared_transactions. -// In the test output, we won't be able to explicitly observe this since -// recover_prepared_transactions unblocks citus_create_restore_point after in-progress prepared transactions -// are recovered. +// verify that citus_create_restore_point is blocked by concurrent recover_prepared_transactions permutation "s1-begin" "s1-recover" "s2-create-restore" "s1-commit" // verify that citus_create_restore_point is blocked by concurrent DROP TABLE