public inbox for [email protected]
help / color / mirror / Atom feedFrom: vignesh C <[email protected]>
To: Nisha Moond <[email protected]>
Cc: Amit Kapila <[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: Tue, 4 Feb 2025 16:41:47 +0530
Message-ID: <CALDaNm2o8L+CdYLwemYR5GiMZtuz_bBH7c-0XQtTDwauSFd3Sw@mail.gmail.com> (raw)
In-Reply-To: <CABdArM5Y7wt2t77nfo58Fv00py7h=vLk-rb7QxtBZPnDmxE=bw@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>
On Tue, 4 Feb 2025 at 15:58, Nisha Moond <[email protected]> wrote:
>
> Here are the v68 patches, incorporating above as well as comments from [1].
>
Few comments:
1) Let's call TimestampDifferenceExceedsSeconds only if
idle_replication_slot_timeout_mins is set to avoid the
TimestampDifferenceExceedsSeconds function call and timestamp diff
calculation if not required:
+ if (CanInvalidateIdleSlot(s) &&
+ TimestampDifferenceExceedsSeconds(s->inactive_since, now,
+ idle_replication_slot_timeout_mins * SECS_PER_MINUTE))
+ {
+ invalidation_cause = cause;
+ inactive_since = s->inactive_since;
+ }
+ break;
2) Let's keep the prototype after TimestampDifferenceExceeds to keep
it consistent with the source file and will also make it easy to
search:
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index d26f023fb8..e1d05d6779 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -143,5 +143,8 @@ extern int date2isoyear(int year, int mon, int mday);
extern int date2isoyearday(int year, int mon, int mday);
extern bool TimestampTimestampTzRequiresRewrite(void);
+extern bool TimestampDifferenceExceedsSeconds(TimestampTz start_time,
+
TimestampTz stop_time,
+
int threshold_sec);
3)How about we change the below:
+#ifdef USE_INJECTION_POINTS
+
+ /*
+ * To test idle
timeout slot invalidation, if the
+ * slot-time-out-inval
injection point is attached,
+ * set inactive_since
to a very old timestamp (1
+ * microsecond since
epoch) to immediately invalidate
+ * the slot.
+ */
+ if
(IS_INJECTION_POINT_ATTACHED("slot-time-out-inval"))
+ s->inactive_since = 1;
+#endif
to:
#ifdef USE_INJECTION_POINTS
/*
* To test idle timeout slot invalidation, if the
* slot-time-out-inval injection point is attached,
* set inactive_since to current time and invalidate the slot immediately.
*/
if (IS_INJECTION_POINT_ATTACHED("slot-time-out-inval") &&
idle_replication_slot_timeout_mins)
{
invalidation_cause = cause;
inactive_since = s->inactive_since = now;
}
#else
/*
* Check if the slot needs to be invalidated due to
* idle_replication_slot_timeout GUC.
*/
if (TimestampDifferenceExceedsSeconds(s->inactive_since, now,
idle_replication_slot_timeout_mins * SECS_PER_MINUTE))
{
invalidation_cause = cause;
inactive_since = s->inactive_since;
}
#endif
We can just invalidate the slot directly without checking the time
difference if idle_replication_slot_timeout_mins is set and
inactive_since can hold the now value.
Regards,
Vignesh
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: <CALDaNm2o8L+CdYLwemYR5GiMZtuz_bBH7c-0XQtTDwauSFd3Sw@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