Horton

Re: Conflict detection for update_deleted in logical replication

Nisha Moond <nisha.moond412@gmail.com>

View thread View on postgr.es
From: Nisha Moond <nisha.moond412@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, shveta malik <shveta.malik@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-01-08T07:48:37Z
Lists: pgsql-hackers 
On Tue, Jan 7, 2025 at 6:04 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
>
> Attached the V19 patch which addressed comments in [1][2][3][4][5][6][7].
>

Here are a couple of initial review comments on v19 patch set:

1) The subscription option 'retain_conflict_info' remains set to
"true" for a subscription even after restarting the server with
'track_commit_timestamp=off', which can lead to incorrect behavior.
  Steps to reproduce:
   1. Start the server with 'track_commit_timestamp=ON'.
   2. Create a subscription with (retain_conflict_info=ON).
   3. Restart the server with 'track_commit_timestamp=OFF'.

 - The apply worker starts successfully, and the subscription retains
'retain_conflict_info=true'. However, in this scenario, the
update_deleted conflict detection will not function correctly without
'track_commit_timestamp'.
```
postgres=# show track_commit_timestamp;
 track_commit_timestamp
------------------------
 off
(1 row)

postgres=# select subname, subretainconflictinfo from pg_subscription;
 subname | subretainconflictinfo
---------+-----------------------
 sub21   | t
 sub22   | t
```

2) With the new parameter name change to "retain_conflict_info", the
error message for both the 'CREATE SUBSCRIPTION' and 'ALTER
SUBSCRIPTION' commands needs to be updated accordingly.

  postgres=# create subscription sub11 connection 'dbname=postgres'
publication pub1 with (retain_conflict_info=on);
  ERROR:  detecting update_deleted conflicts requires
"track_commit_timestamp" to be enabled
  postgres=# alter subscription sub12 set (retain_conflict_info=on);
  ERROR:  detecting update_deleted conflicts requires
"track_commit_timestamp" to be enabled

 - Change the message to something similar - "retaining conflict info
requires "track_commit_timestamp" to be enabled".

--
Thanks,
Nisha

Commits

  1. Fix intermittent BF failures in 035_conflicts.

  2. Resume conflict-relevant data retention automatically.

  3. Fix intermittent test failure introduced in 6456c6e2c4.

  4. Fix Coverity issue reported in commit a850be2fe.

  5. Add test to prevent premature removal of conflict-relevant data.

  6. Post-commit review fixes for 228c370868.

  7. Add max_retention_duration option to subscriptions.

  8. Detect and report update_deleted conflicts.

  9. Preserve conflict-relevant data during logical replication.

  10. Integrate FullTransactionIds deeper into two-phase code

  11. Improve checks for GUC recovery_target_timeline

  12. Prevent excessive delays before launching new logrep workers.

  13. Keep WAL segments by slot's last saved restart LSN

  14. Rework some code handling pg_subscription data in psql and pg_dump

  15. Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.