postgres.git / summary / log / commit / refs

commit    01cc92fa6251c4d1a0ac128806db6f7d8f31ec7f
Author:   Amit Kapila <akapila@postgresql.org>
Date:     Mon Dec 11 03:06:17 2023 +0000
First released in: 17.0, 12.18, 13.14, 14.11, 15.6, 16.2

    Fix an undetected deadlock due to apply worker.
    
    The apply worker needs to update the state of the subscription tables to
    'READY' during the synchronization phase which requires locking the
    corresponding subscription. The apply worker also waits for the
    subscription tables to reach the 'SYNCDONE' state after holding the locks
    on the subscription and the wait is done using WaitLatch. The 'SYNCDONE'
    state is changed by tablesync workers again by locking the corresponding
    subscription. Both the state updates use AccessShareLock mode to lock the
    subscription, so they can't block each other. However, a backend can
    simultaneously try to acquire a lock on the same subscription using
    AccessExclusiveLock mode to alter the subscription. Now, the backend's
    wait on a lock can sneak in between the apply worker and table sync worker
    causing deadlock.
    
    In other words, apply_worker waits for tablesync worker which waits for
    backend, and backend waits for apply worker. This is not detected by the
    deadlock detector because apply worker uses WaitLatch.
    
    The fix is to release existing locks in apply worker before it starts to
    wait for tablesync worker to change the state.
    
    Reported-by: Tomas Vondra
    Author: Shlok Kyal
    Reviewed-by: Amit Kapila, Peter Smith
    Backpatch-through: 12
    Discussion: https://postgr.es/m/d291bb50-12c4-e8af-2af2-7bb9bb4d8e3e@enterprisedb.com


 src/backend/replication/logical/tablesync.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index fcc6e1f311c..81fff1919d7 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -540,15 +540,25 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
 					/* Now safe to release the LWLock */
 					LWLockRelease(LogicalRepWorkerLock);
 
+					if (started_tx)
+					{
+						/*
+						 * We must commit the existing transaction to release
+						 * the existing locks before entering a busy loop.
+						 * This is required to avoid any undetected deadlocks
+						 * due to any existing lock as deadlock detector won't
+						 * be able to detect the waits on the latch.
+						 */
+						CommitTransactionCommand();
+						pgstat_report_stat(false);
+					}
+
 					/*
 					 * Enter busy loop and wait for synchronization worker to
 					 * reach expected state (or die trying).
 					 */
-					if (!started_tx)
-					{
-						StartTransactionCommand();
-						started_tx = true;
-					}
+					StartTransactionCommand();
+					started_tx = true;
 
 					wait_for_relation_state_change(rstate->relid,
 												   SUBREL_STATE_SYNCDONE);


[parent: 8ca56620caff]