public inbox for [email protected]  

help / color / mirror / Atom feed

From: shveta malik <[email protected]>
To: Imran Zaheer <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: effective_wal_level is not decreasing after using REPACK (CONCURRENTLY)
Date: Mon, 25 May 2026 09:41:38 +0530
Message-ID: <CAJpy0uCV9L+D4kevxHGtxHKoOCnpdmW-3j4ssDbFtycThFC02A@mail.gmail.com> (raw)
In-Reply-To: <CA+UBfakjtK=Dh9NQTf7g5F+eMoDbEQUSa8BN6RBGPFzGr3H-sA@mail.gmail.com>
References: <CA+UBfaktds57dw2M8BEv_kS-=ixph3w+3MxKixtaDQMi_k7Ybg@mail.gmail.com>
	<CAJpy0uDmBEHJAk7i5afYaqGA8V8j1XdfAT_=WYwZ=AhHZu6Z1A@mail.gmail.com>
	<CAD21AoBSUi58=aL0uRPO57d8Qjrd_cakGeuguDEy+fPRKRLh2Q@mail.gmail.com>
	<CAJpy0uDzMzyfc2VOt+H1B96mjRXjgJ8oVLatxkNjPHzpHuQifA@mail.gmail.com>
	<CA+UBfakjtK=Dh9NQTf7g5F+eMoDbEQUSa8BN6RBGPFzGr3H-sA@mail.gmail.com>

On Sat, May 23, 2026 at 2:49 PM Imran Zaheer <[email protected]> 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

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: effective_wal_level is not decreasing after using REPACK (CONCURRENTLY)
  In-Reply-To: <CAJpy0uCV9L+D4kevxHGtxHKoOCnpdmW-3j4ssDbFtycThFC02A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox