public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: vignesh C <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
Date: Wed, 5 Feb 2025 14:42:31 +0530
Message-ID: <CAA4eK1K+g3rWY+Y4PyOGsjJ_EdBLBD1L3=H8B9BAfejCF_OoUw@mail.gmail.com> (raw)
In-Reply-To: <CALDaNm0X_vgAxKPT+c14yqKcgE5-x4XBdXsCAVqD6_aa-QYUvg@mail.gmail.com>
References: <CALDaNm39dtWHoQej2wd74zYGphA0e54Rx6aTRoCEnMXZ5ysbow@mail.gmail.com>
	<CABdArM45=j=DAFTEZA8p=oqYo-XqRwhaPCdKpciQCJx+jNeTJg@mail.gmail.com>
	<TYAPR01MB5692B7687EE7981AA91BA5B9F5362@TYAPR01MB5692.jpnprd01.prod.outlook.com>
	<CABdArM4N60XShbmxXgz4z8ENQGPOMhXSKzvmskdv8V=mQsYOOA@mail.gmail.com>
	<CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s+rh4nHpWeJaBQ2F+mg@mail.gmail.com>
	<CAHut+Psi-mso-qSyMkHRP8e+psDFFmMp4NZTiL_yrFWidHkGMw@mail.gmail.com>
	<CALDaNm3znxKvTv=MDmMBOSk6XurkKgYyh9cH49NLL7SmJ62Q_A@mail.gmail.com>
	<CABdArM7CPR0Ag6OgtufATUn+x=Q_h3cdBcc39r+xeKYDks5Mfw@mail.gmail.com>
	<CALDaNm1FQS04aG0C0gCRpvi-o-OTdq91y6Az34YKN-dVc9r5Ng@mail.gmail.com>
	<CABdArM7LJopsdDyBAhjbsW3K-Yq=f75bnjaU5HRSsTkGWXibaQ@mail.gmail.com>
	<CAHut+PvfSHzKgXJTd=PbJieaXFgR9+VhhR3+eb7jCFFj6kC5bA@mail.gmail.com>
	<CABdArM6qPsZE9v7qymNTwiRt68DgOJ+LsAMhweTxZx1xr1cVOA@mail.gmail.com>
	<CAA4eK1JO+3Q+wkyjhMK+h_5MveTLFY7q2KRbrg1ojtO6dLCaMw@mail.gmail.com>
	<CABdArM4wO2gPfcjrFWWL=D18PyFeWJZJcJGY3uJMzqL+4LDGpw@mail.gmail.com>
	<CAHut+Pt7hYB3kzNgmKJ16z46p9M+4U94k=b=kydPhmZtCtBYHA@mail.gmail.com>
	<CAA4eK1+YyUWkR6WvNzVjgU+QTYtLFo0uofr5d3DKwkLcaOfAsQ@mail.gmail.com>
	<CABdArM6FKY6GjVu=7ak8CAYNK0e329h-JEVa3p0UE5niwP_=yA@mail.gmail.com>
	<CABdArM5oXMO63uETfEFLEan3UfNBiDwprdoyYYdeWP9fsYNg9A@mail.gmail.com>
	<CAHut+PsrSLh3+fSyYWBgHvoPmES1w3RS9xzAQ8_iFLfOqDbF4A@mail.gmail.com>
	<CAA4eK1KT9F6v6CZOZhr9mo8v32yWwYxeNpAc=1aXgpd5KBHtzQ@mail.gmail.com>
	<CABdArM7D9u1an6gzfArAL32Jn0QQkKs7JffUxcZ9EqzAaGrfvQ@mail.gmail.com>
	<CANhcyEWgb8WquGM34xHCknAQxVVp0yYBCsOtEgo5r0Y9aimFAw@mail.gmail.com>
	<CAA4eK1Kw=vZ2FZ4DdrmOhuxOAL=2abaBm8hu_PsVN2Hd6UFP-w@mail.gmail.com>
	<CABdArM5Y7wt2t77nfo58Fv00py7h=vLk-rb7QxtBZPnDmxE=bw@mail.gmail.com>
	<CALDaNm2o8L+CdYLwemYR5GiMZtuz_bBH7c-0XQtTDwauSFd3Sw@mail.gmail.com>
	<CABdArM4JAO+SoEvpJ+h1SiYUrwJPjGcavJS9+LV3788GWKvukA@mail.gmail.com>
	<CALDaNm0X_vgAxKPT+c14yqKcgE5-x4XBdXsCAVqD6_aa-QYUvg@mail.gmail.com>

On Wed, Feb 5, 2025 at 10:30 AM vignesh C <[email protected]> wrote:
>
> On Tue, 4 Feb 2025 at 19:56, Nisha Moond <[email protected]> wrote:
> >
> > Here is v69 patch set addressing above and Kuroda-san's comments in [1].
>
> Few minor suggestions:
> 1) In the slot invalidation reporting below:
> +               case RS_INVAL_IDLE_TIMEOUT:
> +                       Assert(inactive_since > 0);
> +
> +                       /* translator: second %s is a GUC variable name */
> +                       appendStringInfo(&err_detail, _("The slot's
> idle time %s exceeds the configured \"%s\" duration."),
> +
> timestamptz_to_str(inactive_since),
> +
> "idle_replication_slot_timeout");
> +                       /* translator: %s is a GUC variable name */
> +                       appendStringInfo(&err_hint, _("You might need
> to increase \"%s\"."),
> +
> "idle_replication_slot_timeout");
>
> It is logged like:
> 2025-02-05 10:04:11.616 IST [330567] DETAIL:  The slot's idle time
> 2025-02-05 10:02:49.131631+05:30 exceeds the configured
> "idle_replication_slot_timeout" duration.
>
> Here even though we tell idle time, we are logging the inactive_since
> value which kind of gives a wrong meaning.
>
> How about we change it to:
> The slot has been inactive since 2025-02-05 10:02:49.131631+05:30,
> which exceeds the configured "idle_replication_slot_timeout" duration.
>

Would it address your concern if we write the actual idle duration
(now - inactive_since) instead of directly using inactive_since in the
above message?

A few other comments:
1.
+ * 4. The slot is not being synced from the primary while the server
+ *    is in recovery
+ *
+ * Note that the idle timeout invalidation mechanism is not
+ * applicable for slots on the standby server that are being synced
+ * from the primary server (i.e., standby slots having 'synced' field 'true').
+ * Synced slots are always considered to be inactive because they don't
+ * perform logical decoding to produce changes.

The 4th point in the above comment and the rest of the comment is
mostly saying the same thing.

2.
+ * Flush all replication slots to disk. Also, invalidate obsolete slots during
+ * non-shutdown checkpoint.
  *
  * It is convenient to flush dirty replication slots at the time of checkpoint.
  * Additionally, in case of a shutdown checkpoint, we also identify the slots
@@ -1924,6 +2007,45 @@ CheckPointReplicationSlots(bool is_shutdown)

Can we try and see how the patch looks if we try to invalidate the
slot due to idle time at the same time when we are trying to
invalidate due to WAL?

-- 
With Regards,
Amit Kapila.






view thread (50+ messages)  latest in thread

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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
  In-Reply-To: <CAA4eK1K+g3rWY+Y4PyOGsjJ_EdBLBD1L3=H8B9BAfejCF_OoUw@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