MIME-Version: 1.0
References: <CAD21AoAd0BX0L17YxSCbSNbj=3V7dvLVGY0WhwLHAt724rNCZA@mail.gmail.com>
 <ZgulmKzdqxuBuhCU@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAJpy0uBthb-RG=Xg+mM-G6Xy01JLcBsizJuz+xbti08qP_W7AQ@mail.gmail.com>
 <CALj2ACVfDBcbybgyga7ia_KiQPAfEi_3=VDOGf2VU8RkJZSQpw@mail.gmail.com>
 <Zg0QgarM9UlRutLw@ip-10-97-1-34.eu-west-3.compute.internal>
 <CALj2ACUmp-F+irHYfNhmo_9yaCo7aGi7ZB20KQcDR3S9eT56hg@mail.gmail.com>
 <Zg1WqRS/gQrLG2vl@ip-10-97-1-34.eu-west-3.compute.internal>
 <CALj2ACUBiRLHF7VYD-0nYVHSvVBSCQ_5OMbP4tE8Ys_rpvQ--A@mail.gmail.com>
 <Zg2DgfXorAXrC8lw@ip-10-97-1-34.eu-west-3.compute.internal>
 <CALj2ACXbUzp-9tixeUK_dws5k+ARJaXxj8cj9W3adA0XNUS4Hg@mail.gmail.com>
 <Zg+rvmqjL3DfbJdx@ip-10-97-1-34.eu-west-3.compute.internal>
 <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> <CAJpy0uC8Dg-0JS3NRUwVUemgz5Ar2v3_EQQFXyAigWSEQ8U47Q@mail.gmail.com>
In-Reply-To: <CAJpy0uC8Dg-0JS3NRUwVUemgz5Ar2v3_EQQFXyAigWSEQ8U47Q@mail.gmail.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sun, 8 Sep 2024 17:25:42 +0530
Message-ID: <CALj2ACWqLneo2kqP5DEUwDXyHoM5zPn2vE+27oB+Cse9Rvzo_A@mail.gmail.com>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
To: shveta malik <shveta.malik@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, 
	Ajin Cherian <itsajin@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, 
	Masahiko Sawada <sawada.mshk@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CALj2ACWqLneo2kqP5DEUwDXyHoM5zPn2vE%2B27oB%2BCse9Rvzo_A%40mail.gmail.com>
Precedence: bulk

Hi,

Thanks for reviewing.

On Tue, Sep 3, 2024 at 3:01=E2=80=AFPM shveta malik <shveta.malik@gmail.com=
> wrote:
>
> 1)
> I see that ReplicationSlotAlter() will error out if the slot is
> invalidated due to timeout. I have not tested it myself, but do you
> know if  slot-alter errors out for other invalidation causes as well?
> Just wanted to confirm that the behaviour is consistent for all
> invalidation causes.

Will respond to Amit's comment soon.

> 2)
> When a slot is invalidated, and we try to use that slot, it gives this ms=
g:
>
> ERROR:  can no longer get changes from replication slot "mysubnew1_2"
> DETAIL:  The slot became invalid because it was inactive since
> 2024-09-03 14:23:34.094067+05:30, which is more than 600 seconds ago.
> HINT:  You might need to increase "replication_slot_inactive_timeout.".
>
> Isn't HINT misleading? Even if we increase it now, the slot can not be
> reused again.
>
> Below is one side effect if inactive_since keeps on changing:
>
> postgres=3D# SELECT * FROM pg_replication_slot_advance('mysubnew1_1',
> pg_current_wal_lsn());
> ERROR:  can no longer get changes from replication slot "mysubnew1_1"
> DETAIL:  The slot became invalid because it was inactive since
> 2024-09-04 10:03:56.68053+05:30, which is more than 10 seconds ago.
> HINT:  You might need to increase "replication_slot_inactive_timeout.".
>
> postgres=3D# select now();
>                now
> ---------------------------------
>  2024-09-04 10:04:00.26564+05:30
>
> 'DETAIL' gives wrong information, we are not past 10-seconds. This is
> because inactive_since got updated even in ERROR scenario.
>
> ERROR:  can no longer get changes from replication slot "mysubnew1_1"
> DETAIL:  The slot became invalid because it was inactive since
> 2024-09-04 10:06:38.980939+05:30, which is more than 129600 seconds
> ago.
> postgres=3D# select now();
>                now
> ----------------------------------
>  2024-09-04 10:07:35.201894+05:30
>
> I feel we should change this message itself.

Removed the hint and corrected the detail message as following:

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.")));

> 3)
> When the slot is invalidated, the' inactive_since' still keeps on
> changing when there is a subscriber trying to start replication
> continuously. I think ReplicationSlotAcquire() keeps on failing and
> thus Release keeps on setting it again and again. Shouldn't we stop
> setting/chnaging  'inactive_since' once the slot is invalidated
> already, otherwise it will be misleading.
>
> postgres=3D# select failover,synced,inactive_since,invalidation_reason
> from pg_replication_slots;
>
>  failover | synced |          inactive_since          | invalidation_reas=
on
> ----------+--------+----------------------------------+------------------=
---
>  t        | f      | 2024-09-03 14:23:.. | inactive_timeout
>
> after sometime:
>  failover | synced |          inactive_since          | invalidation_reas=
on
> ----------+--------+----------------------------------+------------------=
---
>  t        | f      | 2024-09-03 14:26:..| inactive_timeout

Changed it to not update inactive_since for slots invalidated due to
inactive timeout.

> 4)
> src/sgml/config.sgml:
>
> 4a)
> + A value of zero (which is default) disables the timeout mechanism.
>
> Better will be:
> A value of zero (which is default) disables the inactive timeout
> invalidation mechanism .

Changed.

> 4b)
> 'synced' and inactive_since should point to pg_replication_slots:
>
> example:
> <link linkend=3D"view-pg-replication-slots">pg_replication_slots</link>.<=
structfield>synced</structfield>

Modified.

> 5)
> src/sgml/system-views.sgml:
> + ..the slot has been inactive for longer than the duration specified
> by replication_slot_inactive_timeout parameter.
>
> Better to have:
> ..the slot has been inactive for a time longer than the duration
> specified by the replication_slot_inactive_timeout parameter.

Changed it to the following to be consistent with the config.sgml.

          <literal>inactive_timeout</literal> means that the slot has been
          inactive for longer than the amount of time specified by the
          <xref linkend=3D"guc-replication-slot-inactive-timeout"/> paramet=
er.

Please find the v45 patch posted upthread at
https://www.postgresql.org/message-id/CALj2ACWXQT3_HY40ceqKf1DadjLQP6b1r%3D=
0sZRh-xhAOd-b0pA%40mail.gmail.com
for the changes.

--=20
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com