Feedback-ID: ie3de48e3:Fastmail
Date: Thu, 22 Jan 2026 19:58:05 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Antonin Houska <ah@cybertec.at>
Cc: Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
	Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org
Subject: Re: Race conditions in logical decoding
Message-ID: <202601221804.nbrvjquik7qp@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <124728.1769077978@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/202601221804.nbrvjquik7qp%40alvherre.pgsql>
Precedence: bulk

On 2026-Jan-22, Antonin Houska wrote:

> Mihail Nikalayeu <mihailnikalayeu@gmail.com> wrote:
> 
> > Hello, Andres.
> > 
> > On Tue, Jan 20, 2026 at 6:50 PM Andres Freund <andres@anarazel.de> wrote:
> > > I don't think that's enough - during non-timetravel visibility semantics, you
> > > can only look at the clog if the transaction isn't marked as in-progress in
> > > the procarray.  ISTM that we need to do that here too?
> > 
> > Do you mean replace
> > > if (unlikely(!TransactionIdDidCommit(builder->committed.xip[i])))
> > to
> > > if (unlikely(TransactionIdIsInProgress(builder->committed.xip[i]) || !TransactionIdDidCommit(builder->committed.xip[i])))
> 
> This way the synchronous replication gets stuck, as it did when I tried to use
> XactLockTableWait(): subscriber cannot confirm replication of certain LSN
> because publisher is not able to even finalize the commit (due to the waiting
> for the subscriber's confirmation), and therefore publisher it's not able to
> decode the data and send it to the subscriber.

The layering here is wild, but if I understand it correctly, these XIDs
are all added to an array by SnapBuildAddCommittedTxn(), which in turn
is only called by SnapBuildCommitTxn(), which is only called by
DecodeCommit(), which is only called by xact_decode() when it sees a
XLOG_XACT_COMMIT or XLOG_XACT_COMMIT_PREPARED record by reading WAL.

Crucially, RecordTransactionCommit() writes the WAL first, then marks
everything as committed in CLOG, and finally does the waiting for
the synchronous replica to ACK the commit if necessary.  However, the
transaction is only removed from procarray after RecordTransactionCommit
has returned.

This means that DecodeCommit() could add a transaction to the
SnapBuilder (that needs to be waited for) while that transaction is
still shown as running in ProcArray.  This sounds problematic in itself,
so I'm wondering whether we should do anything (namely, wait) on
DecodeCommit() instead of hacking SnapBuildBuildSnapshot() to patch it
up by waiting after the fact.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/