Re: Improving connection scalability: GetSnapshotData()
Andres Freund <andres@anarazel.de>
Hi, On 2020-03-28 18:39:32 -0700, Peter Geoghegan wrote: > I have heard quite a few complaints about the scalability of snapshot > acquisition in Postgres. Generally from very large users that are not > well represented on the mailing lists, for a variety of reasons. The > GetSnapshotData() bottleneck is a *huge* problem for us. (As problems > for Postgres users go, I would probably rank it second behind issues > with VACUUM.) Yea, I see it similarly. For busy databases, my experience is that vacuum is the big problem for write heavy workloads (or the write portion), and snapshot scalability the big problem for read heavy oltp workloads. > This scalability improvement is clearly very significant. There is > little question that this is a strategically important enhancement for > the Postgres project in general. I hope that you will ultimately be > able to commit the patchset before feature freeze. I've done a fair bit of cleanup, but I'm still fighting with how to implement old_snapshot_threshold in a good way. It's not hard to get it back to kind of working, but it requires some changes that go into the wrong direction. The problem basically is that the current old_snapshot_threshold implementation just reduces OldestXmin to whatever is indicated by old_snapshot_threshold, even if not necessary for pruning to do the specific cleanup that's about to be done. If OldestXmin < threshold, it'll set shared state that fails all older accesses. But that doesn't really work well with approach in the patch of using a lower/upper boundary for potentially valid xmin horizons. I thinkt he right approach would be to split TransactionIdLimitedForOldSnapshots() into separate parts. One that determines the most aggressive horizon that old_snapshot_threshold allows, and a separate part that increases the threshold after which accesses need to error out (i.e. SetOldSnapshotThresholdTimestamp()). Then we can only call SetOldSnapshotThresholdTimestamp() for exactly the xids that are removed, not for the most aggressive interpretation. Unfortunately I think that basically requires changing HeapTupleSatisfiesVacuum's signature, to take a more complex parameter than OldestXmin (to take InvisibleToEveryoneState *), which quickly increases the size of the patch. I'm currently doing that and seeing how the result makes me feel about the patch. Alternatively we also can just be less efficient and call GetOldestXmin() more aggressively when old_snapshot_threshold is set. That'd be easier to implement - but seems like an ugly gotcha. Greetings, Andres Freund
Commits
-
Try to unbreak 021_row_visibility.pl on mingw.
- 1df2b50dbebb 14.0 landed
-
Fix and test snapshot behavior on standby.
- 7b28913bcab8 14.0 landed
-
Fix race condition in snapshot caching when 2PC is used.
- 07f32fcd23ac 14.0 cited
-
snapshot scalability: cache snapshots using a xact completion counter.
- 623a9ba79bbd 14.0 landed
-
Fix use of wrong index in ComputeXidHorizons().
- f6661d3df228 14.0 landed
-
Make vacuum a bit more verbose to debug BF failure.
- 49967da65aec 14.0 landed
-
snapshot scalability: Introduce dense array of in-progress xids.
- 941697c3c1ae 14.0 landed
-
snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags.
- 5788e258bb26 14.0 landed
-
snapshot scalability: Move subxact info to ProcGlobal, remove PGXACT.
- 73487a60fc10 14.0 landed
-
snapshot scalability: Move PGXACT->xmin back to PGPROC.
- 1f51c17c68d0 14.0 landed
-
snapshot scalability: Don't compute global horizons while building snapshots.
- dc7420c2c927 14.0 landed
-
BRIN: Handle concurrent desummarization properly
- 1f42d35a1d61 14.0 cited
-
Track latest completed xid as a FullTransactionId.
- 3bd7f9969a24 14.0 landed
-
Rename VariableCacheData.nextFullXid to nextXid.
- fea10a64340e 14.0 landed
-
snapshot scalability: Move delayChkpt from PGXACT to PGPROC.
- 75848bc74411 13.0 landed
-
Report progress of CREATE INDEX operations
- ab0dfc961b6a 12.0 cited