On 2025/07/09 18:21, Laurenz Albe wrote:
> 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.
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.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
