On Mon, Jan 27, 2025 at 4:20 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jan 27, 2025 at 11:00 AM Nisha Moond <nisha.moond...@gmail.com> wrote: > > > > I discussed the above comments further with Peter off-list, and here > > are the v63 patches with the following changes: > > patch-001: The Assert and related comments have been updated for clarity. > > > > The 0001 patch should be discussed in a separate thread as those are > general improvements that are useful even without the main patch we > are trying to achieve in this thread. I suggest we break it into three > patches (a) Ensure the same inactive_since time for all slots, (b) > Raise an error for invalid slots during ReplicationSlotAcquire(); tell > in the commit message, without this patch when such an ERROR would > have otherwise occurred, and (c) Changes in > InvalidatePossiblyObsoleteSlot(), I suggest to leave this change for > later as this impacts the core logic of invalidation. >
I have started a new thread for these general improvements and have separated the changes (a) and (b) into different patches. You can find the new thread at [1]. > * > @@ -812,7 +823,7 @@ ReplicationSlotAlter(const char *name, const bool > *failover, > Assert(MyReplicationSlot == NULL); > Assert(failover || two_phase); > > - ReplicationSlotAcquire(name, false); > + ReplicationSlotAcquire(name, false, false); > > Why don't we want to give ERROR during Alter? I think it is okay to > not give ERROR for invalid slots during Drop as we are anyway removing > such slots. > Because ReplicationSlotAlter() already handles errors immediately after acquiring the slot. It raises errors for invalidated slots and also raises a different error message if the slot is a physical one. So, In case of ALTER, I feel it is okay to acquire the slot first without raising errors and then handle errors in the pre-defined way. Similar immediate error handling is not available at other places. [1] https://www.postgresql.org/message-id/CABdArM6pBL5hPnSQ%2B5nEVMANcF4FCH7LQmgskXyiLY75TMnKpw%40mail.gmail.com -- Thanks, Nisha
