public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: shveta malik <[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: Mon, 16 Sep 2024 16:54:40 +0530
Message-ID: <CAA4eK1JyGE=zi58zpt_xq_vkobpu8n1TOVKVJ6XJ+0+gisvU9g@mail.gmail.com> (raw)
In-Reply-To: <CALj2ACXjkM4nEE5dXeV=NA2NmbE3AmonKEgudYBMqU3qO9-9fw@mail.gmail.com>
References: <CAD21AoAd0BX0L17YxSCbSNbj=3V7dvLVGY0WhwLHAt724rNCZA@mail.gmail.com>
<[email protected]>
<CAJpy0uBthb-RG=Xg+mM-G6Xy01JLcBsizJuz+xbti08qP_W7AQ@mail.gmail.com>
<CALj2ACVfDBcbybgyga7ia_KiQPAfEi_3=VDOGf2VU8RkJZSQpw@mail.gmail.com>
<[email protected]>
<CALj2ACUmp-F+irHYfNhmo_9yaCo7aGi7ZB20KQcDR3S9eT56hg@mail.gmail.com>
<Zg1WqRS/[email protected]>
<CALj2ACUBiRLHF7VYD-0nYVHSvVBSCQ_5OMbP4tE8Ys_rpvQ--A@mail.gmail.com>
<[email protected]>
<CALj2ACXbUzp-9tixeUK_dws5k+ARJaXxj8cj9W3adA0XNUS4Hg@mail.gmail.com>
<[email protected]>
<CALj2ACW8SYiRL8vGAhez=FdrzxO0W5pVFvN8GF7yKCO8_c=QdQ@mail.gmail.com>
<CAA4eK1+-ipXB+LuUiDNQ5L+sWJFpzK89k-Oc7xOG37AefrptoQ@mail.gmail.com>
<CALj2ACWhTY9mRc7sOpuao=GGidRjCVXc0SNtJ_DNman4nh5CcQ@mail.gmail.com>
<CALj2ACVY+Fd5vC0VjW=5VDK9mmt-Y+PDZxnBp8ngGAZc24Vv9g@mail.gmail.com>
<CALj2ACX0FVrx9f0c2g19Rb-WAJhm3wdyttde3YQtkotL6bUveA@mail.gmail.com>
<CALj2ACXQNGXokgx8APwdxrG4MHMF=cOz6XQtUL7EHua9oUfkgA@mail.gmail.com>
<CAFPTHDZPOc7pY4E0zJ2om0MZLMt+UuiX-VLsJyRO_aSyR8DsnA@mail.gmail.com>
<CALj2ACVu0EJ+QQ7W0oHbL_+hCtk+p-00MNhE4hfjFcXaMepMfg@mail.gmail.com>
<CAA4eK1KVbicrQQbMLEQiHEi9=VJpxrErrK7CiMamwaMPfff3WA@mail.gmail.com>
<CALj2ACXe8+xSNdMXTMaSRWUwX7v61Ad4iddUwnn=djSwx3GLLg@mail.gmail.com>
<CAHut+PuFzCHPCiZbpoQX59kgZbebuWT0gR0O7rOe4t_sdYu=OA@mail.gmail.com>
<CALj2ACUJg2faYd-_Wr3NWoj_GtbW5Bje3G9+o595GyEhcBkbrg@mail.gmail.com>
<CAHut+PuC7nwuT3+36zsLxtBAcUHrSaysrHb0_dxTXqMnFBKZBA@mail.gmail.com>
<CALj2ACWXQT3_HY40ceqKf1DadjLQP6b1r=0sZRh-xhAOd-b0pA@mail.gmail.com>
<CAHut+PtJxxaVhjDCFPqE_V63VaAN_bL+PHRMak=dkghoGrZc0Q@mail.gmail.com>
<CALj2ACXjkM4nEE5dXeV=NA2NmbE3AmonKEgudYBMqU3qO9-9fw@mail.gmail.com>
On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
<[email protected]> wrote:
>
> Please find the attached v46 patch having changes for the above review
> comments and your test review comments and Shveta's review comments.
>
-ReplicationSlotAcquire(const char *name, bool nowait)
+ReplicationSlotAcquire(const char *name, bool nowait, bool error_if_invalid)
{
ReplicationSlot *s;
int active_pid;
@@ -615,6 +620,22 @@ retry:
/* We made this slot active, so it's ours now. */
MyReplicationSlot = s;
+ /*
+ * An error is raised if error_if_invalid is true and the slot has been
+ * previously invalidated due to inactive timeout.
+ */
+ if (error_if_invalid &&
+ s->data.invalidated == RS_INVAL_INACTIVE_TIMEOUT)
+ {
+ Assert(s->inactive_since > 0);
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("can no longer get changes from replication slot \"%s\"",
+ NameStr(s->data.name)),
+ errdetail("This slot has been invalidated because it was inactive
for longer than the amount of time specified by \"%s\".",
+ "replication_slot_inactive_timeout")));
+ }
Why raise the ERROR just for timeout invalidation here and why not if
the slot is invalidated for other reasons? This raises the question of
what happens before this patch if the invalid slot is used from places
where we call ReplicationSlotAcquire(). I did a brief code analysis
and found that for StartLogicalReplication(), even if the error won't
occur in ReplicationSlotAcquire(), it would have been caught in
CreateDecodingContext(). I think that is where we should also add this
new error. Similarly, pg_logical_slot_get_changes_guts() and other
logical replication functions should be calling
CreateDecodingContext() which can raise the new ERROR. I am not sure
about how the invalid slots are handled during physical replication,
please check the behavior of that before this patch.
--
With Regards,
Amit Kapila.
view thread (98+ 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]
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
In-Reply-To: <CAA4eK1JyGE=zi58zpt_xq_vkobpu8n1TOVKVJ6XJ+0+gisvU9g@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