Sorry, but I'm still not convinced this is the right path here. I just want to put my concerns about this approach in a single reply, including some that I do not think were really addressed earlier.
My concern is that this changes the semantics of a row in pg_stat_progress_vacuum in a way that feels pretty confusing. The same row now mixes command-level aggregate fields with backend-local fields, and many of the command-level fields are effectively no-op on worker rows. In the output I am looking at, heap_blks_total, heap_blks_scanned, heap_blks_vacuumed, index_vacuum_count, max_dead_tuple_bytes, dead_tuple_bytes, num_dead_item_ids, indexes_total, indexes_processed, and delay_time are all really leader-level command state. On the other hand, current_index_relid and index_blks_* are backend-local. mode and started_by are unset on worker rows. So to me a row no longer represents one coherent kind of progress information. We do not have precedent for this elsewhere in the progress views, and I think it will be confusing for users and monitoring tools. Documenting the caveats does not really solve that problem. I also worry about the precedent this sets for other progress views that may want worker reporting later, for example pg_stat_progress_create_index. If we applied the same pattern there, we would immediately get more no-op fields on worker rows. So I do not think this is just a VACUUM specific awkwardness. Putting my monitoring-tool hat on, I do not see how this is a cleaner interface. If the worker detail lives in the same view, a tool now has to reconstruct one logical VACUUM by grouping or self-joining pg_stat_progress_vacuum on (datid, relid), and then also know which fields are command-level aggregates and which are backend-local. If we really want to separate worker-level detail from leader-level progress, I think a secondary view with an explicit join key such as leader_pid is a cleaner approach. There is also a broader problem with this approach. If we want to add more worker-specific fields, the view gets wider, which will also add to the incoherence of it. I do not think we need to solve this part immediately, because it is not really a problem yet. But I still think it is worth calling out that the underlying progress tracking only gives us 20 generic slots in st_progress_param[]. This patch works today because it uses slots that were previously unused for VACUUM, but we will run out of those if we want to expose more worker detail. At that point, I think we need a clear mechanism for re-using progress fields differently on leader and worker rows, and I do not think we have that. -- Sami Imseih Amazon Web Services (AWS)
