Re: Add index scan progress to pg_stat_progress_vacuum
Sami Imseih <simseih@amazon.com>
From: "Imseih (AWS), Sami" <simseih@amazon.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>, vignesh C <vignesh21@gmail.com>, "Nathan
Bossart" <nathandbossart@gmail.com>, Robert Haas <robertmhaas@gmail.com>, "Bossart, Nathan" <bossartn@amazon.com>, Peter Geoghegan <pg@bowt.ie>, Justin
Pryzby <pryzby@telsasoft.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-02-18T02:46:27Z
Lists: pgsql-hackers
Attachments
- v24-0001-Add-2-new-columns-to-pg_stat_progress_vacuum.-Th.patch (application/octet-stream) patch v24-0001
Thanks for the review! > + <row> > + <entry><literal>ParallelVacuumFinish</literal></entry> > + <entry>Waiting for parallel vacuum workers to finish index > vacuum.</entry> > + </row> > This change is out-of-date. That was an oversight. Thanks for catching. > Total number of indexes that will be vacuumed or cleaned up. This > number is reported as of the beginning of the vacuuming indexes phase > or the cleaning up indexes phase. This is cleaner. I was being unnecessarily verbose in the original description. > Number of indexes processed. This counter only advances when the phase > is vacuuming indexes or cleaning up indexes. I agree. > Also, index_processed sounds accurate to me. What do you think? At one point, II used index_processed, but decided to change it. "processed" makes sense also. I will use this. > I think these settings are not necessary since the pcxt is palloc0'ed. Good point. > Assert(pvs->pcxt->parallel_progress_callback_arg) looks wrong to me. > If 'arg' is NULL, a SEGV happens. Correct, Assert(pvs) is all that is needed. > I think it's better to update pvs->shared->nindexes_completed by both > leader and worker processes who processed the index. No reason for that, since only the leader process can report process to backend_progress. > I think it's better to make the function type consistent with the > existing parallel_worker_main_type. How about > parallel_progress_callback_type? Yes, that makes sense. > I've attached a patch that incorporates the above comments and has > some suggestions of updating comments etc. I reviewed and incorporated these changes, with a slight change. See v24. - * Increase and report the number of index. Also, we reset the progress - * counters. + * Increase and report the number of index scans. Also, we reset the progress + * counters. Thanks -- Sami Imseih Amazon Web Services (AWS)
Commits
-
Report index vacuum progress.
- 46ebdfe164c6 17.0 landed
-
Add new parallel message type to progress reporting.
- f1889729dd3a 17.0 landed
-
Remove MaxBackends variable in favor of GetMaxBackends() function.
- aa64f23b0292 15.0 cited