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)");
> > >
> > > 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.
>
> I can understand both concerns. One benefit not to use the injection point is
> that any machines can run the test. Currently some BF machines do not enable
> the injection point, thus the feature has not been verified on these env.
> However, unconditional sleep makes the test longer.
>
> 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 code may be
> bit complex, but below codes was enough on my environment.
>
> ```
> # 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;');
>
> $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)");
> }
> ```
>
> 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
