Re: Improving connection scalability: GetSnapshotData()

From: Andres Freund <andres(at)anarazel(dot)de>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Geoghegan <pg(at)bowt(dot)ie>, Bruce Momjian <bruce(at)momjian(dot)us>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: Improving connection scalability: GetSnapshotData()
Date: 2020-07-24 01:11:43
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2020-07-15 21:33:06 -0400, Alvaro Herrera wrote:
> On 2020-Jul-15, Andres Freund wrote:
> > It could make sense to split the conversion of
> > VariableCacheData->latestCompletedXid to FullTransactionId out from 0001
> > into is own commit. Not sure...
> +1, the commit is large enough and that change can be had in advance.

I've done that in the attached.

I wonder if somebody has an opinion on renaming latestCompletedXid to
latestCompletedFullXid. That's the pattern we already had (cf
nextFullXid), but it also leads to pretty long lines and quite a few
comment etc changes.

I'm somewhat inclined to remove the "Full" out of the variable, and to
also do that for nextFullXid. I feel like including it in the variable
name is basically a poor copy of the (also not great) C type system. If
we hadn't made FullTransactionId a struct I'd see it differently (and
thus incompatible with TransactionId), but we have ...

> Note you forward-declare struct GlobalVisState twice in heapam.h.

Oh, fixed, thanks.

I've also fixed a correctness bug that Thomas's cfbot found (and he
personally pointed out). There were occasional make check runs with
vacuum erroring out. That turned out to be because it was possible for
the horizon used to make decisions in heap_page_prune() and
lazy_scan_heap() to differ a bit. I've started a thread about my
concerns around the fragility of that logic [1]. The code around that
can use a bit more polish, I think. I mainly wanted to post a new
version so that the patch separated out above can be looked at.


Andres Freund


Attachment Content-Type Size
v12-0001-Track-latest-completed-xid-as-a-FullTransactionI.patch text/x-diff 20.8 KB
v12-0002-snapshot-scalability-Don-t-compute-global-horizo.patch text/x-diff 122.2 KB
v12-0003-snapshot-scalability-Move-PGXACT-xmin-back-to-PG.patch text/x-diff 20.3 KB
v12-0004-snapshot-scalability-Introduce-dense-array-of-in.patch text/x-diff 50.3 KB
v12-0005-snapshot-scalability-Move-PGXACT-vacuumFlags-to-.patch text/x-diff 18.9 KB
v12-0006-snapshot-scalability-Move-subxact-info-to-ProcGl.patch text/x-diff 26.2 KB
v12-0007-snapshot-scalability-cache-snapshots-using-a-xac.patch text/x-diff 11.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-07-24 01:17:07 Re: [PATCH] audo-detect and use -moutline-atomics compilation flag for aarch64
Previous Message Soumyadeep Chakraborty 2020-07-24 00:58:18 Re: [Patch] ALTER SYSTEM READ ONLY