| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> |
| Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Sami Imseih <samimseih(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com> |
| Subject: | Re: Report index currently being vacuumed in pg_stat_progress_vacuum |
| Date: | 2026-09-15 22:28:00 |
| Message-ID: | CALj2ACXabJxqApcYA-ghLuhg3iryyi-9GHVFwXh27B3TRWSnLQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Sep 15, 2026 at 10:11 AM Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> wrote:
>
> As far as v6: The code looks overall good to me, but I have some
> comments.
Thanks for reviewing.
> 1/
>
> I do think it will be better to do one more split of v6-0001 to separate
> current_index_relid and index_blks_*; they are 2 distinct features. Also
> you can fold the 0002 into the index_blks_* commit.
I prefer to keep them as one patch as I see them as closely related,
but I'm open to splitting if anyone thinks otherwise.
> Also, there are some comment updates still needed:
> 2/
>
> + <structfield>index_blks_done</structfield>. The remaining columns track
> + command-level heap progress that only the leader maintains; they read as
> + zero on worker rows.
>
> We should mention mode and started_by as NULL, such as:
>
> ".... read as zero on worker rows, except that
> <structfield>mode</structfield> and
> <structfield>started_by</structfield> appear as <literal>NULL</literal>."
>
> 3/
>
> + working. A worker row with a <literal>NULL</literal>
> + <structfield>phase</structfield> is a worker that has been launched but has
> + not started on an index yet, which also happens when every index was
> + claimed by another participant before this worker got to it. Because a
> + table can be vacuumed by only one
>
> I don't think phase will ever be NULL here since phase will be set to
> "initializing" at minimum.
>
> 4/
>
> /*
> * Perform work within a launched parallel process.
> *
> * Since parallel vacuum workers perform only index vacuum or index cleanup,
> * we don't need to report progress information.
> */
> void
> parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
> {
>
> This comment is now out-of-date and should be updated.
I agree with all three comments above and have updated the v6 patches
accordingly. Please have a look.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Report-per-index-vacuum-progress-in-pg_stat_progr.patch | application/x-patch | 17.8 KB |
| v7-0002-Remove-IndexVacuumInfo.report_progress.patch | application/x-patch | 4.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | surya poondla | 2026-09-15 22:34:28 | Re: Introduce XID age based replication slot invalidation |
| Previous Message | Bharath Rupireddy | 2026-09-15 22:22:00 | Re: Support for 8-byte TOAST values, round two |