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