Re: Fix 035_standby_logical_decoding.pl race conditions

Hi,

On Tue, Apr 08, 2025 at 02:13:20PM +0900, Michael Paquier wrote:
> On Tue, Apr 08, 2025 at 02:00:35AM +0000, Hayato Kuroda (Fujitsu) wrote:
> > Your patch looks good to me and it could pass on my env. PSA patches for PG16.
> > Patch for PG17 is not changed, just renamed.
>
> @@ -1287,6 +1288,17 @@ LogStandbySnapshot(void)
>
> Assert(XLogStandbyInfoActive());
>
> +#ifdef USE_INJECTION_POINTS
> + if (IS_INJECTION_POINT_ATTACHED("skip-log-running-xacts"))
> + {
> + /*
> + * This record could move slot's xmin forward during decoding, leading
> + * to unpredictable results, so skip it when requested by the test.
> + */
> + return GetInsertRecPtr();
> + }
> +#endif
>
> I have unfortunately not been able to pay much attention to this
> thread, but using an injection point as a trick to disable the
> generation of these random standby snapshot records is an interesting
> approach to stabilize the test, and it should make it faster as well.
> Nice.

Yeah. That said I still think it could be useful to implement what has been
proposed in v1-0001 in [1] i.e:

- A new injection_points_wakeup_detach() function that is holding the spinlock
during the whole duration to ensure that no process can wait in between the
wakeup and the detach.

- injection_wait() should try to reuse an existing slot (if any) before trying
to use an empty one.

This is not needed here anymore (as we're using another injection point that the
one initially prooposed) but I'll open a dedicated thread for that for 19 when
the timing will be appropriate.

[1]: https://www.postgresql.org/message-id/Z6oQXc8LmiTLfwLA%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com