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. > > Thank you for considering some ideas. As I mentioned above, we might > need to consider a case like where 'CREATE SUBSCRIPTION .. > (retain_conflict_info = true)' is rolled back. Having said that, this > comment is just for simplifying the logic. If using TransactionId > instead makes other parts complex, it would not make sense. I'm okay > with leaving this part and improving the comment for > oldest_nonremovable_xid, say, by mentioning that there is a window for > XID wraparound happening between workers computing their > oldst_nonremovable_xid and pg_conflict_detection slot being created. > Fair enough. Let us see what you think about my above response first. -- With Regards, Amit Kapila.
