Horton

Re: Conflict detection for update_deleted in logical replication

shveta malik <shveta.malik@gmail.com>

View thread View on postgr.es
From: shveta malik <shveta.malik@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Amit Kapila <amit.kapila16@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>, Nisha Moond <nisha.moond412@gmail.com>, shveta malik <shveta.malik@gmail.com>
Date: 2025-08-14T10:34:39Z
Lists: pgsql-hackers 
Few more comments:

1)
src/sgml/monitoring.sgml:

+      <para>
+       True if <link
linkend="sql-createsubscription-params-with-retain-dead-tuples"><literal>retain_dead_tuples</literal></link>
+       is enabled and the duration for which information useful for conflict
+       detection is retained by this apply worker does not exceed
+       <link linkend="sql-createsubscription-params-with-max-conflict-retention-duration"><literal>max_conflict_retention_duration</literal></link>;
NULL for
+       parallel apply workers and table synchronization workers.
+      </para></entry>

How about:
True for apply workers if retain_dead_tuples is enabled and the
conflict detection information retention time is within
max_conflict_retention_duration; NULL for parallel apply and table
synchronization workers.

(I have used 'conflict detection information' here, as suggested by
Amit in another email. But if we plan to stick to another version in
LOG message, please do the same here as well)


2)
create_subscription.sgml: retain_dead_tuples:

          If set to <literal>true</literal>, the detection of
          <xref linkend="conflict-update-deleted"/> is enabled, and a physical
          replication slot named
<quote><literal>pg_conflict_detection</literal></quote>
          created on the subscriber to prevent the information for detecting
          conflicts from being removed.

'created on the subscriber' --> 'is created on the subscriber'.
'is' missing.  It belongs to the committed patch, but shall we change
it here?

3)
+          Maximum duration that the apply worker, according to this
subscription,
+          is allowed to retain the information useful for conflict
detection when
+          <literal>retain_dead_tuples</literal> is enabled for the associated
+          subscriptions.

Shall we say:
'Maximum duration for which this subscription's apply worker is
allowed to retain...'.

4)

if (MyLogicalRepWorker->stop_conflict_info_retention &&
!MySubscription->maxconflretention)
{
resume_conflict_info_retention(rdt_data);
return;
}

The above logic is there in all phases beginning except 'request
status'. Do you think we can shift it to
maybe_advance_nonremovable_xid() before we call
process_rdt_phase_transition()? We do not need to 'return' there, it
can proceed with get_candidate_xid immediately in that call itself
once it does reset in resume-call.

thanks
Shveta

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.