On Wed, Jan 8, 2025 at 2:24 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jan 8, 2025 at 2:15 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Tue, Jan 7, 2025 at 2:49 AM Amit Kapila <amit.kapil...@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.
