Re: POC: Unlocked path for GetSnapshotDataReuse

Hi,

On Fri, Oct 09, 2026 at 11:22:38PM +0200, Matthias van de Meent wrote:
> On Fri, 9 Oct 2026 at 14:49, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > I think it might be possible to even do the ->xmin assignment without a lock,
> > but that might take more thought. That'd make it *much* more broadly
> > applicable.
>
> I think I have a theory of operations for this:
>
> 1. Currently, we allow setting xmin without a write-lock.
> Instead, it's allowed even whilst holding LW_SHARED. This means
> there are (should be) no conflicting LW_SHARED readers that may find
> problem with us setting the field whilst they're examining our PGPROC
> entry.
> 2. There are no other backends that examine or update our xmin under
> LW_EXCLUSIVE.
> Only prepared transactions process PGPROC entries that are not
> their own, and those are never proc entries owned by other backends.
>
> So, updating the xmin itself is OK.
>
> Then we need to make sure we don't use the snapshot if a transaction
> commits after we checked the completion count, but before we
> completely updated our xmin. So, once we've inserted the old xmin, we
> have to check that the xmin is still valid with the usual completion
> count test. If that changed, we have to get a new snapshot -- if not,
> then we've just saved ourselves a lock acquisition.

Yeah, this approach makes sense to me. After the first counter check, the unchanged
completion count means that the cached snapshot is still valid, so its xmin is
safe to publish. MyProc->xmin protects the data needed by the snapshot afterwards,
and the second counter check detects a completion between the two checks.

> 0003: Implements full lockless snapshot reuse, even when
> !TransactionIdIsValid(MyProc->xmin).
> This does use pg_atomic_read_membarrier for the reads of
> xactCompletionCount, so that the reads are not reordered relative to
> the other memory reads in the GetSnapshotDataReuse function, but the
> cost of memory-barrier reads should be lower than the cost of the
> writes when we acquire the LwLock.

So we have:

+ curXactCompletionCount = pg_atomic_read_membarrier_u64(
+ &TransamVariables->xactCompletionCount
+ );

and

+xmin_installed:
+ curXactCompletionCount = pg_atomic_read_membarrier_u64(
+ &TransamVariables->xactCompletionCount
+ )

while:

+static inline void
+IncrementXactCompletionCount(void)
+{
+ uint64 xcc;
+
+ Assert(LWLockHeldByMeInMode(ProcArrayLock, LW_EXCLUSIVE));
+
+ xcc = pg_atomic_read_u64(&TransamVariables->xactCompletionCount);
+ xcc += 1;
+ pg_atomic_write_u64(&TransamVariables->xactCompletionCount, xcc);
+}

I've 2 comments here:

=== 1

pg_atomic_read_membarrier_u64() is documented to return the current value when
updates use operations with barrier semantics, while IncrementXactCompletionCount()
uses pg_atomic_write_u64().

IIUC, this is safe because the counter update is followed by the full barrier
LWLockRelease(ProcArrayLock). I could not find another place relying on such a
pairing though: existing callers use barrier semantic writes.

Worth documenting this explicitly?

=== 2

As pg_atomic_read_membarrier_u64() is potentially more expensive than
pg_atomic_read_u64() then, what about keeping pg_atomic_read_membarrier_u64()
at xmin_installed, but using pg_atomic_read_u64() for the first read? Every
successful reuse passes through xmin_installed, so the second read is the final
validation.

But then, the first read may return a stale value, but this should be safe: a
mismatch only causes a fallback, while a match is validated by the membarrier read
at xmin_installed.

This would trade one unconditional pg_atomic_read_membarrier_u64() call for the
possibility of extra work when the relaxed read is stale. Worth benchmarking?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com