Horton

Re: Conflict detection for update_deleted in logical replication

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Nisha Moond <nisha.moond412@gmail.com>, shveta malik <shveta.malik@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-01-09T05:50:21Z
Lists: pgsql-hackers 
On Wed, Jan 8, 2025 at 2:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Jan 8, 2025 at 2:15 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Tue, Jan 7, 2025 at 2:49 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > We thought of another approach, which is to create/drop this slot first as
> > > > soon as one enables/disables detect_update_deleted (E.g. create/drop slot
> > > > during DDL). But it seems complicate to control the concurrent slot
> > > > create/drop. For example, if one backend A enables detect_update_deteled, it
> > > > will create a slot. But if another backend B is disabling the
> > > > detect_update_deteled at the same time, then the newly created slot may be
> > > > dropped by backend B. I thought about checking the number of subscriptions that
> > > > enables detect_update_deteled before dropping the slot in backend B, but the
> > > > subscription changes caused by backend A may not visable yet (e.g. not
> > > > committed yet).
> > > >
> > >
> > > This means that for the transaction whose changes are not yet visible,
> > > we may have already created the slot and the backend B would end up
> > > dropping it. Is it possible that during the change of this new option
> > > via DDL, we take AccessExclusiveLock on pg_subscription as we do in
> > > DropSubscription() to ensure that concurrent transactions can't drop
> > > the slot? Will that help in solving the above scenario?
> >
> > If we create/stop the slot during DDL, how do we support rollback DDLs?
> >
>
> We will prevent changing this setting in a transaction block as we
> already do for slot related case. See use of
> PreventInTransactionBlock() in subscriptioncmds.c.
>

On further thinking, even if we prevent this command in a transaction
block, there is still a small chance of rollback. Say, we created the
slot as the last operation after making database changes, but still,
the transaction can fail in the commit code path. So, it is still not
bulletproof. However, we already create a remote_slot at the end of
CREATE SUBSCRIPTION, so, if by any chance the transaction fails in the
commit code path, we will end up having a dangling slot on the remote
node. The same can happen in the DROP SUBSCRIPTION code path as well.
We can follow that or the other option is to allow creation of the
slot by the backend and let drop be handled by the launcher which can
even take care of dangling slots. However, I feel it will be better to
give the responsibility to the launcher for creating and dropping the
slot as the patch is doing and use the FullTransactionId for each
worker. What do you think?

-- 
With Regards,
Amit Kapila.

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.