Message-ID: <2dcea297694d9ab8eafd1db70d385ec0b49e3d27.camel@cybertec.at>
Subject: Re: Unexpected behavior when setting "idle_replication_slot_timeout"
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, 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 11:21:33 +0200
In-Reply-To: 
 <OSCPR01MB14966D811DB105E49289B8298F549A@OSCPR01MB14966.jpnprd01.prod.outlook.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>
	 <1cea80b2ed4f355ae6a68e36f56a1410b59ea4a8.camel@cybertec.at>
	 <OSCPR01MB14966D811DB105E49289B8298F549A@OSCPR01MB14966.jpnprd01.prod.outlook.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/2dcea297694d9ab8eafd1db70d385ec0b49e3d27.camel%40cybertec.at>
Precedence: bulk

On Wed, 2025-07-09 at 07:24 +0000, Hayato Kuroda (Fujitsu) wrote:
> > > +# 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?
> >=20
> > That is debatable.  I thought that adding two seconds would be worth th=
e
> > advantage of removing the injection point that only fakes the timeout.
> > We'd get better test coverage.
> >=20
> > But if the people who run the regression tests dozens of times every da=
y
> > object to the two seconds, I won't insist on this part of the patch.
>=20
> I can understand both concerns. One benefit not to use the injection poin=
t is
> that any machines can run the test. Currently some BF machines do not ena=
ble
> the injection point, thus the feature has not been verified on these env.
> However, unconditional sleep makes the test longer.
>=20
> Based on that, I want to propose the hybrid approach; use injection_point=
_attach()
> if possible, otherwise wait few seconds. One concern is that the test cod=
e may be
> bit complex, but below codes was enough on my environment.
>=20
> ```
> # Check if the 'injection_points' extension is available, as it may be
> # possible that this script is run with installcheck, where the module
> # would not be installed by default.
> if ($node->check_extension('injection_points'))
> {
> 	# Register an injection point on the node to forcibly cause a slot
> 	# invalidation due to idle_timeout
> 	$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
>=20
> 	$node->safe_psql('postgres',
> 		"SELECT injection_points_attach('slot-timeout-inval', 'error');");
> }
> else
> {
> 	# Otherwise wait a bit for the replication slots to become invalid
> 	$node->safe_psql('postgres', "SELECT pg_sleep(2)");
> }
> ```
>=20
> Attached patch implements the idea, which can be applied atop v2. How do =
you think?

I have no objections.

I'll leave the decision to Fujii Masao or whoever wants to commit something
in this area.

Yours,
Laurenz Albe