agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Rui Zhao <zhaorui126@gmail.com>
Cc: alvherre@kurilemu.de, Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Subject: Re: Race conditions in logical decoding
Date: Thu, 17 Sep 2026 10:37:35 +0200
Message-ID: <8247.1789634255@localhost> (raw)
In-Reply-To: <CAHWVJhHXyLtS-8mdL9WhEWfsERb=FN7JdPD0GYAXgTmCnqbYGw@mail.gmail.com>
References: <202603201543.t6gxppyyk66p@alvherre.pgsql>
<aoiRAEAAzDnXfkDN@alvherre.pgsql>
<CAHWVJhHXyLtS-8mdL9WhEWfsERb=FN7JdPD0GYAXgTmCnqbYGw@mail.gmail.com>
Rui Zhao <zhaorui126@gmail.com> wrote:
> 1. The wait belongs in SnapBuildInitialSnapshot() and nowhere else:
> SnapBuildBuildSnapshot() does not need it, and in
> SnapBuildInitialSnapshot() it can be a wait on the transaction lock.
> 0001 does that.
>
> SnapBuildInitialSnapshot() is the only place where the builder's list of
> committed transactions turns into a regular MVCC snapshot, and it is
> HeapTupleSatisfiesMVCC() on that snapshot that asks CLOG about a
> transaction between xmin and xmax. The historic snapshots that
> SnapBuildBuildSnapshot() hands to the reorder buffer never do:
> HeapTupleSatisfiesHistoricMVCC() decides the range [xmin, xmax) by the
> xip array alone and consults CLOG only below xmin, and builder->xmin is
> always the oldestRunningXid of an xl_running_xacts record, so a
> transaction below it had left the procarray, and so updated CLOG, before
> that record was written.
I initially thought that it's silly to rely on such tricky details, but not
consulting CLOG appears to be a design choice - see the header comment in
snapbuild.c.
* ........ Also, our snapshots need to be different in comparison to normal
* MVCC ones because in contrast to those we cannot fully rely on the clog and
* pg_subtrans for information about committed transactions because they might
* commit in the future from the POV of the WAL entry we're currently
* decoding. ...
And regarding snapshot's xmin, I agree that it's controlled by
xl_running_xacts WAL record and that it does not advance until the transaction
has been recorded in CLOG.
Thus I'm not opposed to the idea that it's enough to add the check to
SnapBuildInitialSnapshot().
> + if (!RecoveryInProgress())
> + {
> + RunningTransactions running;
> + int nrunning;
> +
> + running = GetRunningTransactionData();
> + nrunning = running->xcnt + running->subxcnt;
> + LWLockRelease(ProcArrayLock);
> + LWLockRelease(XidGenLock);
> +
> + for (int i = 0; i < nrunning; i++)
> + {
> + TransactionId running_xid = running->xids[i];
> +
> + if (bsearch(&running_xid, snap->xip, snap->xcnt,
> + sizeof(TransactionId), xidComparator) != NULL)
> + XactLockTableWait(running_xid, NULL, NULL, XLTW_None);
> + }
> + }
I don't understand why you check all transactions in procarray, instead of
only those in snap->xip.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
view thread (38+ messages) latest in thread
Message-ID: <8247.1789634255@localhost>
Permalink: ../8247.1789634255@localhost/
Also on: postgresql.org/message-id/8247.1789634255@localhost
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, zhaorui126@gmail.com, andres@anarazel.de, mihailnikalayeu@gmail.com
Subject: Re: Race conditions in logical decoding
In-Reply-To: <8247.1789634255@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox