From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
---|---|
To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Gunnar Morling <gunnar(dot)morling(at)googlemail(dot)com>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Unexpected behavior when setting "idle_replication_slot_timeout" |
Date: | 2025-07-09 09:21:33 |
Message-ID: | 2dcea297694d9ab8eafd1db70d385ec0b49e3d27.camel@cybertec.at |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2025-07-09 10:19:44 | Re: Unexpected behavior when setting "idle_replication_slot_timeout" |
Previous Message | Michael Paquier | 2025-07-09 07:35:23 | Re: The same 2PC data maybe recovered twice |