Re: display offset along with block number in vacuum errors

On Fri, Aug 14, 2020 at 4:06 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Aug 10, 2020 at 10:24 AM Masahiko Sawada
> <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> >
> > It's true that heap_page_is_all_visible() is called from only
> > lazy_vacuum_page() but I'm concerned it would lead misleading since
> > it's not actually removing tuples but just checking after vacuum. I
> > guess that the errcontext should show what the process is actually
> > doing and therefore help the investigation, so I thought VACUUM_HEAP
> > might not be appropriate for this case. But I see also your point.
> > Other vacuum error context phases match with vacuum progress
> > information phrases. So in heap_page_is_all_visible (), I agree it's
> > better to update the offset number and keep the phase VACUUM_HEAP
> > rather than do nothing.
> >
>
> Okay, I have changed accordingly and this means that the offset will
> be displayed for the vacuum phase as well. Apart from this, I have
> fixed all the comments raised by me in the attached patch. One thing
> we need to think is do we want to set offset during heap_page_prune
> when called from lazy_scan_heap? I think the code path for
> heap_prune_chain is quite deep, so an error can occur in that path.
> What do you think?
>

The reason why I have not included heap_page_prune related change in
the patch is that I don't want to sprinkle this in every possible
function (code path) called via vacuum especially if the probability
of an error in that code path is low. But, I am fine if you and or
others think that it is a good idea to update offset in
heap_page_prune as well.

--
With Regards,
Amit Kapila.