Re: Fix 035_standby_logical_decoding.pl race conditions

On Tue, Apr 1, 2025 at 2:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi Kuroda-san,
>
> On Tue, Apr 01, 2025 at 01:22:49AM +0000, Hayato Kuroda (Fujitsu) wrote:
> > Dear Bertrand,
> >
>
> Thanks for the updated patch!
>
> > > s/to avoid the seeing a xl_running_xacts/to avoid seeing a xl_running_xacts/?
> >
> > Fixed.
>
> hmm, not sure as I still can see:
>
> +# Note that injection_point is used to avoid the seeing the xl_running_xacts
>
> === 1
>
> +                * XXX What value should we return here? Originally this returns the
> +                * inserted location of RUNNING_XACT record. Based on that, here
> +                * returns the latest insert location for now.
> +                */
> +               return GetInsertRecPtr();
>
> Looking at the LogStandbySnapshot() that are using the output lsn, i.e:
>
> pg_log_standby_snapshot()
> BackgroundWriterMain()
> ReplicationSlotReserveWal()
>
> It looks ok to me to use GetInsertRecPtr().
>

+1.

> But if we "really" want to produce a "new" WAL record, what about using
> LogLogicalMessage()?
>

We are using injection points for testing purposes, which means the
caller is aware of skipping the running_xacts record during the test
run. So, there doesn't seem to be any reason to do anything extra.

--
With Regards,
Amit Kapila.