Re: Conflict detection for update_deleted in logical replication

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