Horton

RE: Conflict detection for update_deleted in logical replication

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

View thread View on postgr.es
From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Nisha Moond <nisha.moond412@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, shveta malik <shveta.malik@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-12-11T09:02:19Z
Lists: pgsql-hackers

Attachments

On Wednesday, December 11, 2024 3:34 PM  Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
> 
> On Wednesday, December 11, 2024 1:06 PM Amit Kapila
> <amit.kapila16@gmail.com> wrote:
> > On Fri, Dec 6, 2024 at 1:28 PM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com>
> > wrote:
> > >
> > > On Thursday, December 5, 2024 6:00 PM Amit Kapila
> > <amit.kapila16@gmail.com> wrote:
> > > >
> > > >
> > > > A few more comments:
> > > > 1.
> > > > +static void
> > > > +wait_for_local_flush(RetainConflictInfoData *data)
> > > > {
> > > > ...
> > > > +
> > > > + data->phase = RCI_GET_CANDIDATE_XID;
> > > > +
> > > > + maybe_advance_nonremovable_xid(data);
> > > > +}
> > > >
> > > > Isn't it better to reset all the fields of data before the next
> > > > round of GET_CANDIDATE_XID phase? If we do that then we don't need
> > > > to reset
> > > > data->remote_lsn = InvalidXLogRecPtr; and data->last_phase_at =
> > > > InvalidFullTransactionId; individually in
> > > > request_publisher_status() and
> > > > get_candidate_xid() respectively. Also, it looks clean and logical
> > > > to me unless I am missing something.
> > >
> > > The remote_lsn was used to determine whether a status is received,
> > > so was reset each time in request_publisher_status. To make it more
> > > straightforward, I added a new function parameter 'status_received',
> > > which would be set to true when calling
> > > maybe_advance_nonremovable_xid() on receving the status. After this
> > change, there is no need to reset the remote_lsn.
> > >
> >
> > As part of the above comment, I had asked for three things (a) avoid
> > setting
> > data->remote_lsn = InvalidXLogRecPtr; in request_publisher_status();
> > data->(b)
> > avoid setting data->last_phase_at =InvalidFullTransactionId; in
> > get_candidate_xid(); (c) reset data in
> > wait_for_local_flush() after wait is over. You only did (a) in the
> > patch and didn't mention anything about (b) or (c). Is that intentional? If so,
> what is the reason?
> 
> I think I misunderstood the intention, so will address in next version.
> 
> >
> > *
> > +static bool
> > +can_advance_nonremovable_xid(RetainConflictInfoData *data) {
> > +
> >
> > Isn't it better to make this an inline function as it contains just one check?
> 
> Agreed. Will address in next version.
> 
> >
> > *
> > + /*
> > + * The non-removable transaction ID for a subscription is centrally
> > + * managed by the main apply worker.
> > + */
> > + if (!am_leader_apply_worker())
> >
> > I have tried to improve this comment in the attached.
> 
> Thanks, will check and merge the next version.

Attach the V16 patch set which addressed above comments.

There is a new 0002 patch where I tried to dynamically adjust the interval for
advancing the transaction ID. Instead of always waiting for
wal_receiver_status_interval, we can start with a short interval and increase
it if there is no activity (no xid assigned on subscriber), but not beyond
wal_receiver_status_interval.

The intention is to more effectively advance xid to avoid retaining too much
dead tuples. My colleague will soon share detailed performance data and
analysis related to this enhancement.

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.