Re: Fix 035_standby_logical_decoding.pl race conditions

On Wed, Apr 2, 2025 at 2:06 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi Kuroda-san,
>
> On Wed, Apr 02, 2025 at 07:16:25AM +0000, Hayato Kuroda (Fujitsu) wrote:
>
> As far v4-0001:
>
> === 1
>
> +# would advance an active replication slot's catalog_xmin
>
> s/would/could/? I mean the system also needs to be "slow" enough (so the
> sleep() in the reproducer)
>
> === 2
>
> +# Injection_point is used to avoid seeing an xl_running_xacts even here. In
> +# scenario 5, we verify the case that the backend process detects the page has
> +# enough tuples; thus, page pruning happens. If the record is generated just
> +# before doing on-pruning, the catalog_xmin of the active slot would be
> +# updated; hence, the conflict would not occur.
>
> Not sure we need to explain what scenario 5 does here, but that does not hurt
> if you feel the need.
>
> Also maybe mention the last update in the comment and add some nuance (like
> proposed in === 1), something like?
>
> "
> # Injection_point is used to avoid seeing a xl_running_xacts here. Indeed,
> # if it is generated between the last 2 updates then the catalog_xmin of the active
> # slot could be updated; hence, the conflict could not occur.
> "
>

I have changed it based on your suggestions and made a few other
changes in the comments. Please see attached.

*
+  if (IS_INJECTION_POINT_ATTACHED("log-running-xacts"))

It is better to name the injection point as skip-log-running-xacts as
that will be appropriate based on its usage.

--
With Regards,
Amit Kapila.