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
signature.asc
Description: PGP signature