Re: display offset along with block number in vacuum errors

On Wed, Jul 29, 2020 at 1:09 AM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>
> On Wed, Jul 29, 2020 at 12:35:17AM +0530, Mahendra Singh Thalor wrote:
> > Apart from these, I fixed comments given by Sawada and Michael in the
> > latest patch. Attaching v2 patch for review.
>
> Thanks.
>
> lazy_check_needs_freeze iterates over blocks and this patch changes it to
> update vacrelstats. I think it should do what
> lazy_{vacuum/cleanup}_heap/page/index do and call update_vacuum_error_info() at
> its beginning (even though only the offset is changed), and then
> restore_vacuum_error_info() at its end (to "revert back" to the item number it
> started with).
>

I see that Mahendra has changed patch as per this suggestion but I am
not convinced that it is a good idea to sprinkle
update_vacuum_error_info()/restore_vacuum_error_info() at places more
than required. I see that it might look a bit clean from the
perspective that if tomorrow we use the function
lazy_check_needs_freeze() for a different purpose then we don't need
to worry about the wrong phase information. If we are worried about
that then we should have an assert in that function to ensure that the
current phase is VACUUM_ERRCB_PHASE_SCAN_HEAP.

--
With Regards,
Amit Kapila.