Re: lazy_vacuum_heap()'s removal of HEAPTUPLE_DEAD tuples

On Tue, Jan 8, 2013 at 8:19 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:

>
> At that point in the investigation, I realized that the cost of being able
> to
> remove entire tuples in lazy_vacuum_heap() greatly exceeds the benefit.
> Again, the benefit is being able to remove tuples whose inserting
> transaction
> aborted between the HeapTupleSatisfiesVacuum() call in heap_page_prune()
> and
> the one in lazy_scan_heap(). To make that possible, lazy_vacuum_heap()
> grabs
> a cleanup lock, calls PageRepairFragmentation(), and emits a WAL record for
> every page containing LP_DEAD line pointers or HEAPTUPLE_DEAD tuples. If
> we
> take it out of the business of removing tuples, lazy_vacuum_heap() can skip
> WAL and update lp_flags under a mere shared lock. The second attached
> patch,
> for HEAD, implements that. Besides optimizing things somewhat, it
> simplifies
> the code and removes rarely-tested branches. (This patch supersedes the
> backpatch-oriented patch rather than stacking with it.)
>
>
+1. I'd also advocated removing the line pointer array scan in
lazy_scan_heap() because the heap_page_prune() does most of the work. The
reason why we still have that scan in lazy_scan_heap() is to check for new
dead tuples (which should be rare), check for all-visibility of the page
and freeze tuples if possible. I think we can move all of that to
heap_page_prune().

But while you are at it, I wonder if you have time and energy to look at
the single pass vacuum work that I, Robert and others tried earlier but
failed to get to the final stage [1][2]. If we do something like that, we
might not need the second scan of the heap at all, which as you rightly
pointed out, does not do much today anyway, other than marking LP_DEAD line
pointers to LP_UNUSED. We can push that work to the next vacuum or even a
foreground task.

BTW, with respect to your idea of not WAL logging the operation of setting
LP_DEAD to LP_UNUSED in lazy_vacuum_page(), I wonder if this would create
problems with crash recovery. During normal vacuum operation, you may have
converted LP_DEAD to LP_UNUSED. Then you actually used one of those line
pointers for subsequent PageAddItem() on the page. If you crash after that
but before the page gets written to the disk, during crash recovery, AFAICS
PageAddItem() will complain with "will not overwrite a used ItemId" warning
and subsequent PANIC of the recovery.

Thanks,
Pavan

[1] http://archives.postgresql.org/pgsql-hackers/2011-07/msg00624.php
[2]
http://archives.postgresql.org/message-id/CABOikdPhAX5uGugB9RJNSj+zVEYTV8Sn4ctYfcMBc47r6_B2_g@mail.gmail.com

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee