Re: Nasty bug in heap_page_prune

On Thu, Mar 6, 2008 at 6:53 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> While working on the previously discussed refactoring of
> heap_page_prune, I came to the realization that its use of
> CacheInvalidateHeapTuple is not just a PANIC risk but simply wrong :-(
> The semantics aren't right: inval.c assumes that it's dealing with
> transactional invalidations,

Oh. I did not know that :-(

>
> Not sure about a clean solution to this. I don't really want to
> bastardize inval.c by making it deal with nontransactional semantics,
> but there may be no other way.
>
> Or we could forget about letting VACUUM FULL collapse out LP_REDIRECT
> pointers, at least in system catalogs. That might be the best answer
> for 8.3 in any case; I am not seeing a real fix that I'd care to risk
> back-patching. (Note that this is not exactly trivial in itself, since
> then vacuum.c would need at least some minimal ability to deal with
> LP_REDIRECT entries.)
>

I am not sure how ugly or difficult it would be to teach inval.c to handle
non-transactional invalidations.

But as you said, skipping collapse of LP_REDIRECT pointers may not be
a good idea either. The overhead of 4 bytes per tuple for system
tables may not be much, but handling LP_REDIRECT pointers in vacuum.c
would be cumbersome and error prone. This was very painful before we
added the step to collapse redirected pointers.

We had a stress test to run concurrent INSERTs / UPDATEs / VACUUMs /
VACUUM FULL and CREATE/DROP INDEXes, and VACUUM FULL used to
once in a while complain about tuple mismatch.

Thanks,
Pavan

--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com