agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@kurilemu.de>
Cc: Rui Zhao <zhaorui126@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Subject: Re: Race conditions in logical decoding
Date: Fri, 18 Sep 2026 15:16:32 +0200
Message-ID: <7388.1789737392@localhost> (raw)
In-Reply-To: <aq0sA8aF8q6kTmJM@alvherre.pgsql>
References: <aq0sA8aF8q6kTmJM@alvherre.pgsql>
Alvaro Herrera <alvherre@kurilemu.de> wrote:
> On 2026-Sep-17, Antonin Houska wrote:
>
> > > + 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.
>
> Hmm, but he does: for all the transactions that are running, only those
> that are found by bsearch() in the snap->xip array are waited for. Is
> that not what we want?
>
> I guess we could do it the other way around: iterate for each item on
> snap->xip and search for those in running->xids. Is that what you
> suggest?
>
> We don't know offhand which array is largest; it would be better to
> iterate on the smaller one and bsearch the largest. (Or maybe if both
> are sorted, scan them simultaneously.) I don't find any reference to
> say that running_xid is sorted.
Maybe I miss the point, but what's wrong about modifying the existing loop
that inverts the meaning of the ->xip array
/*
* snapbuild.c builds transactions in an "inverted" manner, which means it
* stores committed transactions in ->xip, not ones in progress. Build a
* classical snapshot by marking all non-committed transactions as
* in-progress. This can be expensive.
*/
for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);)
{
...
}
by calling XactLockTableWait() for each XID we find in the array (i.e. each
committed transaction)?
> I don't understand these two paragraphs:
>
> * A subtransaction is covered by its top-level transaction, which is in
> * snap->xip as well, or was purged from it because it is below xmin and
> * thus finished long ago.
Me neither. AFAIU SnapBuildCommitTxn() adds both top-level transaction and
subtransactions to the builder's array of committed transaction.
> * Historic snapshots do not need this: between xmin and xmax they rely on
> * xip alone, and transactions below xmin had left the procarray by the
> * time the xl_running_xacts record that set xmin was written.
I think this is related to the note that HeapTupleSatisfiesHistoricMVCC() does
not really use CLOG in the 3rd paragraph in [1].
[1] https://www.postgresql.org/message-id/CAHWVJhHXyLtS-8mdL9WhEWfsERb%3DFN7JdPD0GYAXgTmCnqbYGw%40mail.g...
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
view thread (38+ messages) latest in thread
Message-ID: <7388.1789737392@localhost>
Permalink: ../7388.1789737392@localhost/
Also on: postgresql.org/message-id/7388.1789737392@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, alvherre@kurilemu.de, zhaorui126@gmail.com, andres@anarazel.de, mihailnikalayeu@gmail.com
Subject: Re: Race conditions in logical decoding
In-Reply-To: <7388.1789737392@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