Re: HOT chain validation in verify_heapam()
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Peter Geoghegan <pg@bowt.ie>
Cc: pgsql-hackers@lists.postgresql.org, Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Aleksander Alekseev <aleksander@timescale.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2022-11-14T22:33:07Z
Lists: pgsql-hackers
Hi, On 2022-11-14 14:13:10 -0800, Peter Geoghegan wrote: > > I think the problem partially is that the proposed verify_heapam() code is too > > "aggressive" considering things to be part of the same hot chain - which then > > means we have to be very careful about erroring out. > > > > The attached isolationtester test triggers: > > "unfrozen tuple was updated to produce a tuple at offset %u which is frozen" > > "updated version at offset 3 is also the updated version of tuple at offset %u" > > > > Despite there afaict not being any corruption. Worth noting that this happens > > regardless of hot/non-hot updates being used (uncomment s3ci to see). > > Why don't you think that there is corruption? I looked at the state after the test and the complaint is bogus. It's caused by the patch ignoring the cur->xmax == next->xmin condition if next->xmin is FrozenTransactionId. The isolationtester test creates a situation where that leads to verify_heapam() considering tuples to be part of the same chain even though they aren't. > Because I feel like I'm repeating myself more than I should, but: why isn't > it as simple as "HOT chain traversal logic is broken by frozen xmin in the > obvious way, therefore all bets are off"? Because that's irrelevant for the testcase and a good number of my concerns. > Maybe you're right about the proposed new functionality getting things wrong > with your adversarial isolation test, but I seem to have missed the > underlying argument. Are you just talking about regular update chains here, > not HOT chains? Something else? As I noted, it happens regardless of HOT being used or not. The tuples aren't part of the same chain, but the patch treats them as if they were. The reason the patch considers them to be part of the same chain is precisely the FrozenTransactionId condition I was worried about. Just because t_ctid points to a tuple on the same page and the next tuple has xmin == FrozenTransactionId, doesn't mean they're part of the same chain. Once you encounter a tuple with a frozen xmin you simply cannot assume it's part of the chain you've been following. Greetings, Andres Freund
Commits
-
amcheck: Generalize one of the recently-added update chain checks.
- c87aff065c33 16.0 landed
-
amcheck: Tighten up validation of redirect line pointers.
- 80d5e3a61551 16.0 landed
-
amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.
- 8fd5aa76c367 14.8 landed
- 701ec5557968 15.3 landed
- e88754a1965c 16.0 landed
-
amcheck: Fix a few bugs in new update chain validation.
- 949e2e7c4f68 16.0 landed
-
Fix new test case to work on (some?) big-endian architectures.
- c75a623304bc 16.0 landed
-
Don't test HEAP_XMAX_INVALID when freezing xmax.
- 02d647bbf057 16.0 landed