Hi Nisha.
My review comments for patch v58-0001.
======
src/backend/replication/slot.c
InvalidatePossiblyObsoleteSlot:
1.
/*
- * If the slot can be acquired, do so and mark it invalidated
- * immediately. Otherwise we'll signal the owning process, below, and
- * retry.
+ * If the slot can be acquired, do so and mark it as invalidated. If
+ * the slot is already ours, mark it as invalidated. Otherwise, we'll
+ * signal the owning process below and retry.
*/
- if (active_pid == 0)
+ if (active_pid == 0 ||
+ (MyReplicationSlot == s && active_pid == MyProcPid))
{
As you previously explained [1] "This change applies to all types of
invalidation, not just inactive_timeout case [...] It's a general
optimization for the case when the current process is the active PID
for the slot."
In that case, should this be in a separate patch that can be pushed to
master by itself, i.e. independent of anything else in this thread
that is being done for the purpose of implementing the timeout
feature?
======
[1] https://www.postgresql.org/message-id/CABdArM5tcYTQ2zeAPWTciTnea4jj6sPUjVY9M1O-4wWoTBjFgw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Austalia
