From: Antonin Houska <ah@cybertec.at>
To: alvherre@kurilemu.de
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Race conditions in logical decoding
In-reply-to: <202601221950.c5lwdnye3idv@alvherre.pgsql>
References: <202601221950.c5lwdnye3idv@alvherre.pgsql>
Comments: In-reply-to =?us-ascii?Q?=3D=3Futf-8=3FQ=3F=3DC3=3D81lvaro=3F=3D?=
 Herrera <alvherre@kurilemu.de>
   message dated "Thu, 22 Jan 2026 20:59:21 +0100."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Fri, 23 Jan 2026 07:33:32 +0100
Message-ID: <3805.1769150012@localhost>
Archived-At: <https://www.postgresql.org/message-id/3805.1769150012%40localhost>
Precedence: bulk

=C3=81lvaro Herrera <alvherre@kurilemu.de> wrote:

> I think this algorithm is strange -- if you do have to wait more than
> once for one transaction, it would lead to doing the
> TransactionIdDidCommit again times for _all_ transactions by starting
> the inner loop from scratch, which sounds really wasteful.  Why not nest
> the for() loops the other way around?

I'm quite sure I wanted to iterate through committed.xnt in the outer loop,
but probably got distracted by something else and messed things up.

> Something like this perhaps,
>=20
>     for (int i =3D 0; i < builder->committed.xcnt; i++)
>     {
>         for (;;)
>         {
>             if (TransactionIdDidCommit(builder->committed.xip[i]))
>                 break;
>             else
>             {
>                 (void) WaitLatch(MyLatch,
>                                  WL_LATCH_SET, WL_TIMEOUT, WL_EXIT_ON_PM_=
DEATH,
>                                  10L,
>                                  WAIT_EVENT_SNAPBUILD_CLOG);
>                 ResetLatch(MyLatch);
>             }
>             CHECK_FOR_INTERRUPTS();
>         }
>     }
>=20
> This way you wait repeatedly for one transaction until it is marked
> committed; and once it does, you don't test it again.

Sure, that's much beter. Thanks.

--=20
Antonin Houska
Web: https://www.cybertec-postgresql.com