On Wed, Jul 9, 2025 at 8:47 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> 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.
>
I think an additional two seconds is not worth this test, especially
when we have another way to test most of it, and it could be
noticeable overhead for developers who need to run check-world
multiple times in a day. If we want to proceed with changing the
units, we can leave the existing test as it is. BTW, as mentioned
yesterday, I still feel keeping minutes as a unit for
idle_replication_slot_timeout has merits, but I surrender my case
based on what most other people feel is the right thing to do.
> The "hybrid approach" saves the two seconds on system with injection points,
> but cannot simplify the code.
>
I feel that would be a needless complexity in the test.
--
With Regards,
Amit Kapila.
