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. * @@ -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. -- With Regards, Amit Kapila.
