RE: Conflict detection for update_deleted in logical replication

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, shveta malik <shveta.malik@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, vignesh C <vignesh21@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2025-09-01T04:45:42Z
Lists: pgsql-hackers
On Saturday, August 30, 2025 12:48 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> 
> On Fri, Aug 29, 2025 at 11:49 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > Here is the new version patch set which also addressed Shveta's
> comments[1].
> >
> 
> Thanks for the patches here, I tested the v68-001 patch alone, please find
> review comments -

Thanks for the comments!

> 
> 1) If a sub is created with retain_dead_tuples=on but disabled, e.g.
> postgres=# create subscription sub3 CONNECTION 'dbname=postgres
> host=localhost port=8841' PUBLICATION pub3 WITH
> (enabled=false,retain_dead_tuples=on);
> WARNING:  deleted rows to detect conflicts would not be removed until the
> subscription is enabled
> HINT:  Consider setting retain_dead_tuples to false.
> NOTICE:  created replication slot "sub3" on publisher CREATE
> SUBSCRIPTION
> 
> In this case, the conflict slot is not created until the sub is enabled.

I think this is a separate issue that the sub creation command does
not wakeup the launcher to create the slot in time. So, I will prepare a
fix in another thread.

> Also, if the
> slot already exists but all other subscriptions have stopped retaining
> (slot.xmin=NULL), the dead tuple retention will not start until the slot is
> recreated.
> To me, the above warning seems misleading in this case.
> 
> 2) A similar situation can happen with ALTER SUBSCRIPTION. For example,
> consider two subscriptions where retention has stopped for both and slot.xmin
> is NULL:
> 
>  subname | subenabled | subretaindeadtuples | submaxretention |
> subretentionactive
> ---------+------------+---------------------+-----------------+---------
> ---------+------------+---------------------+-----------------+---------
> ---------+------------+---------------------+-----------------+--
>  sub2    | t          | t                   |             100 | f
>  sub1    | t          | t                   |             100 | f
> 
> postgres=# select slot_name,active,xmin from pg_replication_slots ;
>        slot_name       | active | xmin
> -----------------------+--------+------
>  pg_conflict_detection | t      |
> 
> If we try to resume retention only for sub1 by toggling retain_dead_tuples:
> 
> postgres=# alter subscription sub1 set (retain_dead_tuples =off);
> NOTICE:  max_retention_duration is ineffective when retain_dead_tuples is
> disabled ALTER SUBSCRIPTION postgres=# alter subscription sub1 set
> (retain_dead_tuples =on);
> NOTICE:  deleted rows to detect conflicts would not be removed until the
> subscription is enabled ALTER SUBSCRIPTION
> 
> 2a) Here also the retention NOTICE is ambiguous as slot.xmin remains NULL.
> Though, the above steps don't strictly follow the docs (i.e.
> slot should be recreated to resume the retention), still the notice can be
> confusing for users.
> 
> 2b) Also, the retention is not resumed for sub1(expected), but still the
> subretentionactive is changed to true.
> 
>  subname | subenabled | subretaindeadtuples | submaxretention |
> subretentionactive
> ---------+------------+---------------------+-----------------+---------
> ---------+------------+---------------------+-----------------+---------
> ---------+------------+---------------------+-----------------+--
>  sub1    | f          | t                   |             100 | t
>  sub2    | t          | t                   |             100 | f
> 
> I think we should avoid changing subretentionactive to true in such cases until
> the slot is recreated and retention is actually resumed.
> Thoughts?

Since I have added slot recovery functionality to 0001, so I think these comments
should also be addressed.

Best Regards,
Hou zj

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.