Re: error context for vacuum to include block number

On Fri, Dec 13, 2019 at 10:28:50PM +0900, Michael Paquier wrote:

>> v4-0001-Rename-buf-to-avoid-shadowing-buf-of-another-type.patch
>> v4-0002-Remove-redundant-call-to-vacuum-progress.patch
>> v4-0003-deduplication.patch
>> v4-0004-vacuum-errcontext-to-show-block-being-processed.patch
>> v4-0005-add-errcontext-callback-in-lazy_vacuum_heap-too.patch

> What is the purpose of 0001 in the context of this thread?  One could
> say the same about 0002 and 0003.  Anyway, you are right with 0002 as
> the progress value for PROGRESS_VACUUM_PHASE gets updated twice in a
> row with the same value.  So let's clean up that.  

It's related code which I cleaned up before adding new stuff.  Not essential,
thus separate (0002 should be backpatched).

> The refactoring in 0003 is interesting, so I would be tempted to merge
> it.  Now you have one small issue in it:
> -    /*
> -     * Forget the now-vacuumed tuples, and press on, but be careful
> -     * not to reset latestRemovedXid since we want that value to be
> -     * valid.
> -     */
> +     lazy_vacuum_heap_index(onerel, vacrelstats, Irel, nindexes, indstats);
>       vacrelstats->num_dead_tuples = 0;
> -     vacrelstats->num_index_scans++;
> You are moving this comment within lazy_vacuum_heap_index, but it
> applies to num_dead_tuples and not num_index_scans, no?  You should
> keep the incrementation of num_index_scans within the routine though.

Thank you, fixed.

-- 
Justin Pryzby
System Administrator
Telsasoft
+1-952-707-8581