From: Antonin Houska <ah@cybertec.at>
To: alvherre@kurilemu.de
cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
    "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
    Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
    Andres Freund <andres@anarazel.de>
Subject: Re: Race conditions in logical decoding
In-reply-to: <aqEtEZP-7kpX3_Ov@alvherre.pgsql>
References: <aqEtEZP-7kpX3_Ov@alvherre.pgsql>
Comments: In-reply-to =?us-ascii?Q?=3D=3Futf-8=3FQ=3F=3DC3=3D81lvaro=3F=3D?=
 Herrera <alvherre@kurilemu.de>
   message dated "Wed, 09 Sep 2026 12:20:22 +0200."
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Date: Wed, 09 Sep 2026 20:08:11 +0200
Message-ID: <14036.1788977291@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/14036.1788977291%40localhost>
Precedence: bulk

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

> Hello, replying to Hou and Houska emails in one.
>=20
> On 2026-Aug-22, Zhijie Hou (Fujitsu) wrote:
>=20
> > I think the cache might be better placed in the SnapBuild struct (at le=
ast on
> > HEAD) rather than in static variables. As currently written, it persist=
s across
> > decoding sessions in the same backend - a session could build a snapsho=
t, drop
> > the slot, and later create a new slot and build another snapshot, poten=
tially
> > consulting stale entries from the first builder. For example, it has a =
wraparound
> > concern: after XID wrap, a cached value could refer to a different tran=
saction,
> > causing us to skip the CLOG wait and reintroduce the inconsistency this=
 patch
> > aims to fix.
>=20
> Hmm, yeah, there's definitely a problem with that.  Now, this bug can
> affect logical decoding in existing releases as well, so we should
> backpatch this fix, and I'm not sure about changing SnapBuild in
> backbranches.  Maybe another approach is to use file-level statics
> (rather than function-level) so that they can be reset by slot drop
> routines.
>=20
> > Besides, just to confirm one note: IIUC, for exported snapshots by logi=
calrep, a
> > transaction could be treated as committed while still in PGPROC, while
> > concurrent MVCC snapshots still see it as in progress which looks incon=
sistent.
> > I understand that waiting for ProcArray removal in the general case cou=
ld
> > deadlock against synchronous replication, so it's probably acceptable t=
o leave
> > it unchanged for internal usage in active replication processes.
>=20
> OK.  TBH I'm somewhat unease about this inconsistency; I wondered about
> doing the CLOG-based test only in sync replication and using
> XidIsInProgress otherwise, but didn't really try (which is to say: I'm
> not even sure if it's _possible_ at all.)

I'm trying to understand if this kind of inconsistency has the chance to be
seen by users. I suspect the concern is about a session having isolation le=
vel
at least REPEATABLE_READ which scans the table two times using the same
snapshot, however another session runs REPACK (CONCURRENTLY) in between. Due
to the inconsistency explained above, the snapshot might miss some changes
that REPACK already does see.

IMO the 2nd scan will not see a different version of the table the table had
to be locked before the first scan started, so REPACK won't be able to fini=
sh
until the whole transaction is finished. Even w/o keeping the lock between =
the
scans, both scans would retrieve the same rows as long as REPACK
(CONCURRENTLY) is MVCC-safe (currently it's is not, but should be in the
future).

Regarding logical replication, yes, this inconsistency can be the reason so=
me
data changes are already visible on the replica while some snapshots don't =
yet
see it on the primary. However I think that can happen anyway if the MVCC
snapshot for the scan on the primary had been created before the snapshot f=
or
the logical replication.

Maybe I've just misunderstood something.

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