Horton

Re: Improving connection scalability: GetSnapshotData()

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
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-07T18:28:52Z
Lists: pgsql-hackers 
Hi,

On 2020-04-07 10:51:12 -0700, Andres Freund wrote:
> > +void AssertTransactionIdMayBeOnDisk(TransactionId xid)
> > 
> > Formatting.
> > 
> > + * Assert that xid is one that we could actually see on disk.
> > 
> > I don't know what this means. The whole purpose of this routine is
> > very unclear to me.
> 
> It's intended to be a double check against

forgetting things...? Err:

It is intended to make it easier to detect cases where the passed
TransactionId is not safe against wraparound. If there is protection
against wraparound, then the xid

a) may never be older than ShmemVariableCache->oldestXid (since
   otherwise the rel/datfrozenxid could not have advanced past the xid,
   and because oldestXid is what what prevents ->nextFullXid from
   advancing far enough to cause a wraparound)

b) cannot be >= ShmemVariableCache->nextFullXid. If it is, it cannot
   recently have come from GetNewTransactionId(), and thus there is no
   anti-wraparound protection either.

As full wraparounds are painful to exercise in testing,
AssertTransactionIdMayBeOnDisk() is intended to make it easier to detect
potential hazards.

The reason for the *OnDisk naming is that [oldestXid, nextFullXid) is
the appropriate check for values actually stored in tables. There could,
and probably should, be a narrower assertion ensuring that a xid is
protected against being pruned away (i.e. a PGPROC's xmin covering it).

The reason for being concerned enough in the new code to add the new
assertion helper (as well as a major motivating reason for making the
horizons 64 bit xids) is that it's much harder to ensure that "global
xmin" style horizons don't wrap around. By definition they include other
backend's ->xmin, and those can be released without a lock at any
time. As a lot of wraparound issues are triggered by very longrunning
transactions, it is not even unlikely to hit such problems: At some
point somebody is going to kill that old backend and ->oldestXid will
advance very quickly.

There is a lot of code that is pretty unsafe around wraparounds... They
are getting easier and easier to hit on a regular schedule in production
(plenty of databases that hit wraparounds multiple times a week). And I
don't think we as PG developers often don't quite take that into
account.


Does that make some sense? Do you have a better suggestion for a name?

Greetings,

Andres Freund

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