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: Masahiko Sawada <sawada.mshk@gmail.com>, shveta malik <shveta.malik@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-10-02T02:39:21Z
Lists: pgsql-hackers

Attachments

On Tuesday, October 1, 2024 8:44 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Mon, Sep 30, 2024 at 12:02 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Wednesday, September 25, 2024 2:23 AM Masahiko Sawada
> <sawada.mshk@gmail.com> wrote:
> > >
> > > I think the remote wal flush location is asked using a replication protocol.
> > > Therefore, if a new worker is responsible for asking wal flush
> > > location from multiple publishers (like the idea (b)), the
> > > corresponding process would need to be launched on publisher sides
> > > and logical replication would also need to start on each connection.
> > > I think it would be better to get the remote wal flush location
> > > using the existing logical replication connection (i.e., between the
> > > logical wal sender and the apply worker), and advertise the
> > > locations on the shared memory. Then, the central process who holds the
> slot to retain the deleted row versions traverses them and increases slot.xmin if
> possible.
> > >
> > > The cost of requesting the remote wal flush location would not be
> > > huge if we don't ask it very frequently. So probably we can start by
> > > having each apply worker (in the retain_sub_list) ask the remote wal
> > > flush location and can leave the optimization of avoiding sending the
> request for the same publisher.
> >
> > Agreed. Here is the POC patch set based on this idea.
> >
> > The implementation is as follows:
> >
> > A subscription option is added to allow users to specify whether dead
> > tuples on the subscriber, which are useful for detecting
> > update_deleted conflicts, should be retained. The default setting is
> > false. If set to true, the detection of update_deleted will be
> > enabled,
> >
> 
> I find the option name retain_dead_tuples bit misleading because by name one
> can't make out the purpose of the same. It is better to name it as
> detect_update_deleted or something on those lines.
> 
> > and an additional replication
> > slot named pg_conflict_detection will be created on the subscriber to
> > prevent dead tuples from being removed. Note that if multiple
> > subscriptions on one node enable this option, only one replication slot will be
> created.
> >
> 
> In general, we should have done this by default but as detecting
> update_deleted type conflict has some overhead in terms of retaining dead
> tuples for more time, so having an option seems reasonable. But I suggest to
> keep this as a separate last patch. If we can make the core idea work by default
> then we can enable it via option in the end.

Thanks for the comments. I have renamed the option to detect_update_deleted and
make it a separate last patch in the V3 patch set.

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.