Hi, On 2025-03-19 09:17:23 +0200, Heikki Linnakangas wrote: > On 19/03/2025 04:22, Tomas Vondra wrote: > > I kept stress-testing this, and while the frequency massively increased > > on PG18, I managed to reproduce this all the way back to PG14. I see > > ~100x more corefiles on PG18. > > > > That is not a proof the issue was introduced in PG14, maybe it's just > > the assert that was added there or something. Or maybe there's another > > bug in PG18, making the impact worse. > > > > But I'd suspect this is a bug in > > > > commit 623a9ba79bbdd11c5eccb30b8bd5c446130e521c > > Author: Andres Freund <and...@anarazel.de> > > Date: Mon Aug 17 21:07:10 2020 -0700 > > > > snapshot scalability: cache snapshots using a xact completion counter. > > > > Previous commits made it faster/more scalable to compute snapshots. > > But not > > building a snapshot is still faster. Now that GetSnapshotData() does > > not > > maintain RecentGlobal* anymore, that is actually not too hard: > > > > ...
Thanks for debugging and analyzing this! > Looking at the code, shouldn't ExpireAllKnownAssignedTransactionIds() and > ExpireOldKnownAssignedTransactionIds() update xactCompletionCount? This can > happen during hot standby: > > 1. Backend acquires snapshot A with xmin 1000 > 2. Startup process calls ExpireOldKnownAssignedTransactionIds(), > 3. Backend acquires snapshot B with xmin 1050 > 4. Backend releases snapshot A, updating TransactionXmin to 1050 > 5. Backend acquires new snapshot, calls GetSnapshotDataReuse(), reusing > snapshot A's data. > > Because xactCompletionCount is not updated in step 2, the > GetSnapshotDataReuse() call will reuse the snapshot A. But snapshot A has a > lower xmin. I've swapped a lot of the KnownAssigned* code out: Am I right in understanding that the only scenario (when in STANDBY_SNAPSHOT_READY), where ExpireOldKnownAssignedTransactionIds() would "legally" remove a transaction, rather than the commit / abort records doing so, is if the primary crash-restarted while transactions were ongoing? Those transactions won't have a commit/abort records and thus won't trigger ExpireTreeKnownAssignedTransactionIds(), which otherwise would have updated ->xactCompletionCount? When writing the snapshot caching patch, I tried to make sure that all the places that maintain ->latestCompletedXid also update ->xactCompletionCount. Afaict that's still the case. Which, I think, means that we're also missing calls to MaintainLatestCompletedXidRecovery()? If latestCompletedXid is incorrect visibility determinations end up wrong... Greetings, Andres Freund
