On Wed, 2025-07-09 at 19:19 +0900, Fujii Masao wrote:
> On 2025/07/09 18:21, Laurenz Albe wrote:
> > On Wed, 2025-07-09 at 07:24 +0000, Hayato Kuroda (Fujitsu) wrote:
> > > >
> > > 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.
> > >
> > > Attached patch implements the idea, which can be applied atop v2. How do you think?
> >
> > I have no objections.
>
> So the purpose of this "hybrid approach" is to test idle_replication_slot_timeout
> even in environments where the injection point isn't available? Do we really need
> this additional test? It seems to me that the test using the injection point is
> sufficient.
If you think that using the injection point is sufficient, go for it.
That means slightly less test coverage; the injection point skips the call to
TimestampDifferenceExceedsSeconds() to determine if the slot's "inactive_since"
is old enough and just unconditionally returns RS_INVAL_IDLE_TIMEOUT.
But yes, that is not a big loss.
My patch has the advantage of
a) simplifying the code,
b) running the test even if injection points are not available and
c) testing more of the code
at the price of two extra seconds for the TAP tests.
The "hybrid approach" saves the two seconds on system with injection points,
but cannot simplify the code.
I personally can live with all three; my personal preference is my patch.
Yours,
Laurenz Albe
