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

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

  1. Report index vacuum progress.

  2. Add new parallel message type to progress reporting.

  3. Remove MaxBackends variable in favor of GetMaxBackends() function.