Horton

Re: Improving connection scalability: GetSnapshotData()

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Peter Geoghegan <pg@bowt.ie>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Alvaro Herrera <alvherre@2ndquadrant.com>, Jonathan Katz <jkatz@postgresql.org>, Bruce Momjian <bruce@momjian.us>, Thomas Munro <thomas.munro@gmail.com>, David Rowley <dgrowleyml@gmail.com>
Date: 2020-04-07T19:03:46Z
Lists: pgsql-hackers 
On Tue, Apr 7, 2020 at 1:51 PM Andres Freund <andres@anarazel.de> wrote:
> > ComputedHorizons seems like a fairly generic name. I think there's
> > some relationship between InvisibleToEveryoneState and
> > ComputedHorizons that should be brought out more clearly by the naming
> > and the comments.
>
> I don't like the naming of ComputedHorizons, ComputeTransactionHorizons
> much... But I find it hard to come up with something that's meaningfully
> better.

It would help to stick XID in there, like ComputedXIDHorizons. What I
find really baffling is that you seem to have two structures in the
same file that have essentially the same purpose, but the second one
(ComputedHorizons) has a lot more stuff in it. I can't understand why.

> What's the inconsistency? The dropped replication_ vs dropped FullXid
> postfix?

Yeah, just having the member names be randomly different between the
structs. Really harms greppability.

> > Generally, heap_prune_satisfies_vacuum looks pretty good. The
> > limited_oldest_committed naming is confusing, but the comments make it
> > a lot clearer.
>
> I didn't like _committed much either. But couldn't come up with
> something short. _relied_upon?

oldSnapshotLimitUsed or old_snapshot_limit_used, like currentCommandIdUsed?

> It's just adjusting for the changed name of latestCompletedXid to
> latestCompletedFullXid, as part of widening it to 64bits.  I'm not
> really a fan of adding that to the variable name, but surrounding code
> already did it (cf VariableCache->nextFullXid), so I thought I'd follow
> suit.

Oops, that was me misreading the diff. Sorry for the noise.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Try to unbreak 021_row_visibility.pl on mingw.

  2. Fix and test snapshot behavior on standby.

  3. Fix race condition in snapshot caching when 2PC is used.

  4. snapshot scalability: cache snapshots using a xact completion counter.

  5. Fix use of wrong index in ComputeXidHorizons().

  6. Make vacuum a bit more verbose to debug BF failure.

  7. snapshot scalability: Introduce dense array of in-progress xids.

  8. snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags.

  9. snapshot scalability: Move subxact info to ProcGlobal, remove PGXACT.

  10. snapshot scalability: Move PGXACT->xmin back to PGPROC.

  11. snapshot scalability: Don't compute global horizons while building snapshots.

  12. BRIN: Handle concurrent desummarization properly

  13. Track latest completed xid as a FullTransactionId.

  14. Rename VariableCacheData.nextFullXid to nextXid.

  15. snapshot scalability: Move delayChkpt from PGXACT to PGPROC.

  16. Report progress of CREATE INDEX operations