"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > No, I don't think we would ever need to follow a HOT chain past > the aborted tuple. The only thing that worries about this setup though > is the dependency on hint bits being set properly. But the places > where this would be used right now for detecting aborted dead tuples, > apply HeapTupleSatisfiesVacuum on the tuple before checking > for HeapTupleIsHotUpdated, so we are fine.
Practically all the places that check that have just done a tqual.c test, so they can count on the INVALID bits to be up-to-date. If not, it's still OK, it just means that they might uselessly advance to the next (former) chain member. There is always a race condition in these sorts of things: for instance, a tuple could go from INSERT_IN_PROGRESS to DEAD at any instant, if its inserting transaction rolls back. So you have to have adequate defenses in place anyway, like the xmin/xmax comparison. > Or should we just check for XMIN_INVALID explicitly at those places ? I went back and forth on that, but on balance a single macro seems better. Meanwhile I've started looking at the vacuum code, and it seems that v16 has made that part of the patch significantly worse. VACUUM will fail to count tuples that are removed by pruning, which seems like something it should report somehow. And you've introduced a race condition: as I just mentioned, it's perfectly possible that the second call of HeapTupleSatisfiesVacuum gets a different answer than what the prune code saw, especially in lazy VACUUM (in VACUUM FULL it'd suggest that someone released lock early ... but we do have to cope with that). The comments you added seem to envision a more invasive patch that gets rid of the second HeapTupleSatisfiesVacuum pass altogether, but I'm not sure how practical that is, and am not real inclined to try to do it right now anyway ... regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 2: Don't 'kill -9' the postmaster