Message-ID: <1cea80b2ed4f355ae6a68e36f56a1410b59ea4a8.camel@cybertec.at>
Subject: Re: Unexpected behavior when setting "idle_replication_slot_timeout"
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, "David G. Johnston"	
 <david.g.johnston@gmail.com>, Fujii Masao <masao.fujii@oss.nttdata.com>,
 Nisha Moond <nisha.moond412@gmail.com>, Gunnar Morling
 <gunnar.morling@googlemail.com>,  "pgsql-bugs@lists.postgresql.org"	
 <pgsql-bugs@lists.postgresql.org>
Date: Wed, 09 Jul 2025 09:12:51 +0200
In-Reply-To: 
 <CAA4eK1JaYNQtTj5u-YP3G2UJbJoevD4PnUiM+sQy8DerjNPJFA@mail.gmail.com>
References: 
	<CADGJaX_0+FTguWpNSpgVWYQP_7MhoO0D8=cp4XozSQgaZ40Odw@mail.gmail.com>
	 <CABdArM76e8-XChodqKxEiRK3tR_Yv3cymn_AWfP4bOsLDscZAg@mail.gmail.com>
	 <30e91649-06dd-453e-8658-5f4acd03a3bd@oss.nttdata.com>
	 <b26f13d7-769a-4b64-8bed-cb3a833c0604@oss.nttdata.com>
	 <686577f2379e4b978803e72746dcf0dd7baf3937.camel@cybertec.at>
	 <e0789f10-5b48-4d6b-a32c-0f4426d254df@oss.nttdata.com>
	 <CAA4eK1LOyUeCHk3hMtjT-bpqXLAnjODwcO+o7+bTnB2es-fSqw@mail.gmail.com>
	 <70b183fb-f66e-40d8-b0de-be8c1052fa93@oss.nttdata.com>
	 <CAKFQuwYVEgy+fiHMvSoykiEef-VF-ZV9Gxv4rynk9NNK9UMPYQ@mail.gmail.com>
	 <472687.1751731008@sss.pgh.pa.us> <474268.1751731988@sss.pgh.pa.us>
	 <f12271e6aea3b87eaa430d0c9008ca1f3154c31e.camel@cybertec.at>
	 <CAA4eK1JaYNQtTj5u-YP3G2UJbJoevD4PnUiM+sQy8DerjNPJFA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.56.2 (3.56.2-1.fc42) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/1cea80b2ed4f355ae6a68e36f56a1410b59ea4a8.camel%40cybertec.at>
Precedence: bulk

On Wed, 2025-07-09 at 12:05 +0530, Amit Kapila wrote:
> On Tue, Jul 8, 2025 at 10:23=E2=80=AFPM Laurenz Albe <laurenz.albe@cybert=
ec.at> wrote:
> >=20
> > On Sat, 2025-07-05 at 12:13 -0400, Tom Lane wrote:
> > > Oh, wait a second.  I just noticed that this variable is new in v18.
> > > So there won't be a compatibility issue as long as we change it in
> > > v18 too.  So changing it to a base of seconds seems sufficient,
> > > and more in line with existing practice:
> > >=20
> > > Using 'ms' seems clearly overkill, but there's precedent for
> > > timeouts measured in seconds.
> >=20
> > Here is a patch that changes the unit to seconds, like Fujii Masao's
> > patch upthread.  In addition, it removes the now unnecessary injection
> > point for the TAP test and tries to improve the documentation to
> > address my complaints.
> >=20
>=20
> +# Wait a bit for the replication slots to become invalid
> +$node->safe_psql('postgres', "SELECT pg_sleep(2)");
>=20
> Is it a good idea to add 2s to 'make check-world' for one particular test=
?

That is debatable.  I thought that adding two seconds would be worth the
advantage of removing the injection point that only fakes the timeout.
We'd get better test coverage.

But if the people who run the regression tests dozens of times every day
object to the two seconds, I won't insist on this part of the patch.

Yours,
Laurenz Albe