On Mon, Apr 22, 2024 at 9:02 PM shveta malik <shveta.ma...@gmail.com> wrote: > > On Mon, Apr 22, 2024 at 5:10 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Apr 19, 2024 at 1:52 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > > > Please find v9 with the above comments addressed. > > > > > > > I have made minor modifications in the comments and a function name. > > Please see the attached top-up patch. Apart from this, the patch looks > > good to me. > > Thanks for the patch, the changes look good Amit. Please find the merged > patch. >
I've reviewed the patch and have some comments: --- /* - * Early initialization. + * Register slotsync_worker_onexit() before we register + * ReplicationSlotShmemExit() in BaseInit(), to ensure that during the + * exit of the slot sync worker, ReplicationSlotShmemExit() is called + * first, followed by slotsync_worker_onexit(). The startup process during + * promotion invokes ShutDownSlotSync() which waits for slot sync to + * finish and it does that by checking the 'syncing' flag. Thus worker + * must be done with the slots' release and cleanup before it marks itself + * as finished syncing. */ I'm slightly worried that we register the slotsync_worker_onexit() callback before BaseInit(), because it could be a blocker when we want to add more work in the callback, for example sending the stats. --- synchronize_slots(wrconn); + + /* Cleanup the temporary slots */ + ReplicationSlotCleanup(); + + /* We are done with sync, so reset sync flag */ + reset_syncing_flag(); I think it ends up removing other temp slots that are created by the same backend process using pg_create_{physical,logical_replication_slots() function, which could be a large side effect of this function for users. Also, if users want to have a process periodically calling pg_sync_replication_slots() instead of the slotsync worker, it doesn't support a case where we create a temp not-ready slot and turn it into a persistent slot if it's ready for sync. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com