Re: Race conditions in logical decoding

Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:

> Hello, replying to Hou and Houska emails in one.
>
> On 2026-Aug-22, Zhijie Hou (Fujitsu) wrote:
>
> > I think the cache might be better placed in the SnapBuild struct (at least on
> > HEAD) rather than in static variables. As currently written, it persists across
> > decoding sessions in the same backend - a session could build a snapshot, drop
> > the slot, and later create a new slot and build another snapshot, potentially
> > 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 transaction,
> > causing us to skip the CLOG wait and reintroduce the inconsistency this patch
> > aims to fix.
>
> 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.
>
> > Besides, just to confirm one note: IIUC, for exported snapshots by logicalrep, a
> > transaction could be treated as committed while still in PGPROC, while
> > concurrent MVCC snapshots still see it as in progress which looks inconsistent.
> > I understand that waiting for ProcArray removal in the general case could
> > deadlock against synchronous replication, so it's probably acceptable to leave
> > it unchanged for internal usage in active replication processes.
>
> 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 level
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 finish
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 some
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 for
the logical replication.

Maybe I've just misunderstood something.

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