"Pavan Deolasee" <pavan.deolasee@gmail.com> 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
