Re: Fix race condition in InvalidatePossiblyObsoleteSlot()

On Tue, Mar 05, 2024 at 10:17:03AM +0000, Bertrand Drouvot wrote:
> On Tue, Mar 05, 2024 at 09:42:20AM +0900, Michael Paquier wrote:
>> First, in a build where 818fefd8fd is included, this makes the test
>> script a lot slower.  Most of the logic is quick, but we're spending
>> 10s or so checking that catalog_xmin has advanced.  Could it be
>> possible to make that faster?
> 
> Yeah, v2 attached changes this. It moves the injection point after the process
> has been killed so that another process can decode at wish (without the need
> to wait for a walsender timeout) to reach LogicalConfirmReceivedLocation().

Ah, OK.  Indeed that's much faster this way.

> I try to simulate a revert of 818fefd8fd (replacing "!terminated" by "1 == 1"
> before the initial_* assignements).

Yeah.  I can see how this messes up with the calculation of the
conditions, which is enough from my perspective, even if we don't have
any sanity checks in 818fefd8fd originally.

> The issue is that then the new ASSERT is
> triggered leading to the standby shutdown. So, I'm not sure how to improve this
> case.

It's been mentioned recently that we are not good at detecting crashes
in the TAP tests.  I am wondering if we should check the status of the
node in the most popular routines of Cluster.pm and die hard, as one
way to make the tests more responsive..  A topic for a different
thread, for sure.

> Done in v2.

Reworded a few things and applied this version.
--
Michael