Re: Conflict detection for update_deleted in logical replication

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-09-17T00:38:16Z
Lists: pgsql-hackers
On Fri, Sep 13, 2024 at 12:56 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Fri, Sep 13, 2024 at 11:38 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > > >
> > > > So in brief, this solution is only for bidrectional setup? For non-bidirectional,
> > > > feedback_slots is non-configurable and thus irrelevant.
> > >
> > > Right.
> > >
> >
> > One possible idea to address the non-bidirectional case raised by
> > Shveta is to use a time-based cut-off to remove dead tuples. As
> > mentioned earlier in my email [1], we can define a new GUC parameter
> > say vacuum_committs_age which would indicate that we will allow rows
> > to be removed only if the modified time of the tuple as indicated by
> > committs module is greater than the vacuum_committs_age. We could keep
> > this parameter a table-level option without introducing a GUC as this
> > may not apply to all tables. I checked and found that some other
> > replication solutions like GoldenGate also allowed similar parameters
> > (tombstone_deletes) to be specified at table level [2]. The other
> > advantage of allowing it at table level is that it won't hamper the
> > performance of hot-pruning or vacuum in general. Note, I am careful
> > here because to decide whether to remove a dead tuple or not we need
> > to compare its committs_time both during hot-pruning and vacuum.
>
> +1 on the idea,

I agree that this idea is much simpler than the idea originally
proposed in this thread.

IIUC vacuum_committs_age specifies a time rather than an XID age. But
how can we implement it? If it ends up affecting the vacuum cutoff, we
should be careful not to end up with the same result of
vacuum_defer_cleanup_age that was discussed before[1]. Also, I think
the implementation needs not to affect the performance of
ComputeXidHorizons().

> but IIUC this value doesn’t need to be significant; it
> can be limited to just a few minutes. The one which is sufficient to
> handle replication delays caused by network lag or other factors,
> assuming clock skew has already been addressed.

I think that in a non-bidirectional case the value could need to be a
large number. Is that right?

Regards,

[1] https://www.postgresql.org/message-id/20230317230930.nhsgk3qfk7f4axls%40awork3.anarazel.de

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com



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.