Hi, On Thu, Sep 3, 2026 at 9:49 AM Sami Imseih <[email protected]> wrote: > > 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'm studying the patch and discussion so I might be missing something, but let me share my thoughts on this patch: I think it's fine for a progress view to have an entry per backend, including parallel workers, so more than one row for a single command. If we ever implement parallel index rebuilding for REINDEX TABLE, where different workers rebuild different indexes, having one entry per worker in pg_stat_progress_create_index seems the straightforward thing to do. A dedicated view for workers, say pg_stat_progress_create_index_worker, would end up with mostly the same columns as pg_stat_progress_create_index. I can also see Sami's point that the patch makes many of the existing columns of pg_stat_progress_vacuum no-op. But I'm not sure a dedicated worker view really avoids that. If a user wants the progress of one vacuum command they would join the two views, get one row per participant anyway, and the leader's heap columns would be repeated on every row. That's better than reporting them as 0, but the user still has to know which columns are command-level and which are backend-local. It might be worth clarifying the actual query and its output for each approach and comparing them. If we do want a separate view to avoid meaningless columns, I think it should be split by the job, not by who does it. For example a pg_stat_progress_index_vacuum view showing index bulkdelete and index cleanup per index vacuum work, where both the leader and the parallel workers have their own entry. One benefit is that GIN's pending list cleanup invoked from autoanalyze could report there instead of adding more columns to pg_stat_progress_analyze. The same probably applies to REPACK, which might want to use pg_stat_progress_create_index for its index rebuild. This needs infrastructure changes so that a backend can report more than one progress at a time, though, so it isn't something for this patch. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
