On 9/16/07, Tom Lane <[EMAIL PROTECTED]> wrote:
> Something else I was just looking at: in the pruning logic, SetRedirect
> and SetDead operations are done at the same time that we record them for
> the eventual WAL record creation, but SetUnused operations are
> postponed and only done at the end. This seems a bit odd/nonorthogonal.
> Furthermore it's costing extra code complexity: if you were to SetUnused
> immediately then you wouldn't need that bitmap thingy to prevent you
> from doing it twice. I think that the reason it's like that is probably
> because of the problem of potential multiple visits to a DEAD heap-only
> tuple, but it looks to me like that's not really an issue given the
> current form of the testing for aborted tuples, which I have as
> if (HeapTupleSatisfiesVacuum(tuple->t_data, global_xmin, buffer)
> == HEAPTUPLE_DEAD && !HeapTupleIsHotUpdated(tuple))
> heap_prune_record_unused(nowunused, nunused, unusedbitmap,
> ISTM that if HeapTupleIsHotUpdated is false, we could simply SetUnused
> immediately, because any potential chain members after this one must
> be dead too, and they'll get reaped by this same bit of code --- there
> is no need to preserve the chain. (The only case we're really covering
> here is XMIN_INVALID, and any later chain members must certainly be
> XMIN_INVALID as well.) When the HOT-chain is later followed, we'll
> detect chain break anyway, so I see no need to postpone clearing the
> item pointer.
So are you suggesting we go back to the earlier way of handling
aborted tuples separately ? But then we can not do that by simply
checking for !HeaptupleIsHotUpdated. There could be several aborted
tuples at the end of the chain of which all but one are marked HotUpdated.
Or are you suggesting we also check for XMIN_INVALID for detecting
aborted tuples ?
If we handle aborted tuples differently, then we don't need that extra
bitmap and can in fact set the line pointer unused immediately. Otherwise,
as you rightly pointed out, we might break a chain before we prune