From 0d03cc3380d4904b0a29c10fc740b9ea4888bc48 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Thu, 8 Aug 2024 08:06:30 +0000 Subject: [PATCH v4] Change the misleading local end_lsn for prepared transactions. The apply worker was using XactLastCommitEnd as local end_lsn for applying prepare and rollback_prepare. The XactLastCommitEnd value is the end lsn of the last commit applied before the prepare transaction which makes no sense. This LSN is used to decide whether we can send the acknowledgment of the corresponding remote LSN to the server. It is okay not to set the local_end LSN with the actual WAL position for the prepare because we always flush the prepare record. So, we can send the acknowledgment of the remote_end LSN as soon as prepare is finished. The current code is misleading but as such doesn't create any problem, so decided not to backpatch. Author: Hayato Kuroda Reviewed-by: Shveta Malik, Amit Kapila Discussion: https://postgr.es/m/TYAPR01MB5692FA4926754B91E9D7B5F0F5AA2@TYAPR01MB5692.jpnprd01.prod.outlook.com --- src/backend/replication/logical/worker.c | 31 +++++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 6dc54c7283..da4e1e21fa 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -1133,7 +1133,17 @@ apply_handle_prepare(StringInfo s) CommitTransactionCommand(); pgstat_report_stat(false); - store_flush_position(prepare_data.end_lsn, XactLastCommitEnd); + /* + * It is okay not to set the local_end LSN for the prepare because we always + * flush the prepare record. So, we can send the acknowledgment of the + * remote_end LSN as soon as prepare is finished. + * + * XXX For the sake of consistency with commit, we could have set it with + * the LSN of prepare but as of now we don't track that value similar to + * XactLastCommitEnd, and adding it for this purpose doesn't seems worth + * it. + */ + store_flush_position(prepare_data.end_lsn, InvalidXLogRecPtr); in_remote_transaction = false; @@ -1251,7 +1261,12 @@ apply_handle_rollback_prepared(StringInfo s) pgstat_report_stat(false); - store_flush_position(rollback_data.rollback_end_lsn, XactLastCommitEnd); + /* + * It is okay not to set the local_end LSN for the rollback of prepared + * transaction because we always flush the WAL record for it. See + * apply_handle_prepare. + */ + store_flush_position(rollback_data.rollback_end_lsn, InvalidXLogRecPtr); in_remote_transaction = false; /* Process any tables that are being synchronized in parallel. */ @@ -1306,7 +1321,11 @@ apply_handle_stream_prepare(StringInfo s) CommitTransactionCommand(); - store_flush_position(prepare_data.end_lsn, XactLastCommitEnd); + /* + * It is okay not to set the local_end LSN for the prepare because we + * always flush the prepare record. See apply_handle_prepare. + */ + store_flush_position(prepare_data.end_lsn, InvalidXLogRecPtr); in_remote_transaction = false; @@ -1364,7 +1383,11 @@ apply_handle_stream_prepare(StringInfo s) CommitTransactionCommand(); - MyParallelShared->last_commit_end = XactLastCommitEnd; + /* + * It is okay not to set the local_end LSN for the prepare because we + * always flush the prepare record. See apply_handle_prepare. + */ + MyParallelShared->last_commit_end = InvalidXLogRecPtr; pa_set_xact_state(MyParallelShared, PARALLEL_TRANS_FINISHED); pa_unlock_transaction(MyParallelShared->xid, AccessExclusiveLock); -- 2.28.0.windows.1