Thread

Re: effective_wal_level is not decreasing after using REPACK (CONCURRENTLY)

shveta malik <shveta.malik@gmail.com> — 2026-05-25T04:11:38Z

On Sat, May 23, 2026 at 2:49 PM Imran Zaheer <imran.zhir@gmail.com> wrote:
>
> Hi
>
> Thanks for the review. In the attached patch, I added an argument that
> will help explicitly control whether to stop logical decoding or not.
>
> -ReplicationSlotDropAcquired(void)
> +ReplicationSlotDropAcquired(bool disable_logical_decoding)
>
> I hope this will be enough to make the caller intent more explicit and
> will prevent future omissions like this.
>

Overall it looks good. I have a few trivial comments; please
incorporate them if you agree.

1)
+ReplicationSlotDropAcquired(bool disable_logical_decoding)

We can change comments atop this function. Suggestion :

/*
 * Permanently drop the currently acquired replication slot and
 * request the checkpointer to disable logical decoding if requested
 * by the caller.
 */


2)
Additionally I think it will be good to have below sanity check in
ReplicationSlotDropAcquired(). Thoughts?

/* Ensure that slot is logical if caller has requested to disable
logical decoding */
Assert(!disable_logical_decoding || SlotIsLogical(MyReplicationSlot));

3)
@@ -567,7 +567,7 @@ drop_local_obsolete_slots(List *remote_slot_list)
  if (synced_slot)
  {
  ReplicationSlotAcquire(NameStr(local_slot->data.name), true, false);
- ReplicationSlotDropAcquired();
+ ReplicationSlotDropAcquired(false);
  }

Since it is a logical slot and we are still passing false, we may add
a comment. Suggestion:

/*
 * We don't want to disable logical decoding during slot drop on the
 * physical standby, since the standby derives the logical decoding
 * state from the upstream server in the replication chain.
 */

thanks
Shveta