Re: error context for vacuum to include block number (atomic progress update)

On Sun, Dec 29, 2019 at 02:17:47PM -0600, Justin Pryzby wrote:
> The behavior is different from before, but I think that's ok: the number of
> scans is accurate, and the PHASE is accurate, even though it'll change a moment
> later.

pgstat_progress_update_multi_param() is useful when it comes to update
multiple parameters at the same time consistently in a given progress
phase. For example, in vacuum, when beginning the heap scan, the
number of blocks to scan and the max number of dead tuples has to be
updated at the same as the phase name, as things have to be reported
consistently, so that's critical to be consistent IMO. Now, in this
case, we are discussing about updating a parameter which is related to
the index vacuuming phase, while switching at the same time to a
different phase. I think that splitting both is not confusing here
because the number of times vacuum indexes have been done is unrelated
to the heap cleanup happening afterwards. On top of that the new code
is more readable, and future callers of lazy_vacuum_heap() will never
miss to update the progress reporting to the new phase.

While on it, a "git grep -n" is showing me two places where we could
care more about being consistent by using the multi-param version of
progress reports when beginning a new progress phase:
- reindex_index()
- ReindexRelationConcurrently()

One can also note the switch to PROGRESS_VACUUM_PHASE_INDEX_CLEANUP in
lazy_scan_heap() but it can be discarded for the same reason as what
has been refactored recently with the index vacuuming.
--
Michael