Re: Add mode column to pg_stat_progress_vacuum

On Tue, Dec 9, 2025 at 12:22 AM Yu Wang <wangyu_runtime(at)163(dot)com> wrote:
>
> On 12/9/25 5:25 AM, Masahiko Sawada wrote:
> > On Thu, Dec 4, 2025 at 8:30 PM Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
> >> Thank you for the review!
> >>
> >> On Thu, Dec 4, 2025 at 9:15 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >>> I've attached a small change to simplify the 0001 patch. Please review it.
> >> LGTM, and I've updated in v9 patches.
> >>
> >>> Here are a few comments:
> >>>
> >>> + <listitem>
> >>> + <para>
> >>> + <literal>manual</literal>: The analyze was started by an explicit
> >>>
> >>> For consistency with "started_by" in pg_stat_progress_vacuum, I think
> >>> it's better to start with "The operation was started by".
> >> I think "started_by" in pg_stat_progress_vacuum uses "The vacuum was
> >> started by ...".
> > I missed that, you're right.
> >
> >>> ---
> >>> + <command>ANALYZE</command> or <command>VACUUM (ANALYZE)</command>
> >>> + command.
> >>>
> >>> How about using "... or VACUUM with the ANALYZE option"?
> >> Agreed, I've fixed it.
> > Thank you for updating the patch!
> >
> > The patches look good to me, so I'm going to push them if there are
> > not further review comments and objections.
> >
> > Regards,
> >
> Hi,
> I have a few additional suggestions that might be worth considering.

Thank you for the comments!

>
> 1.v9-0001
> ...
> @@ -1018,8 +1018,11 @@ analyze threshold = analyze base threshold +
> analyze scale factor * number of tu
> see <xref linkend="table-lock-compatibility"/>. However, if the
> autovacuum
> is running to prevent transaction ID wraparound (i.e., the
> autovacuum query
> name in the <structname>pg_stat_activity</structname> view ends with
> - <literal>(to prevent wraparound)</literal>), the autovacuum is not
> - automatically interrupted.
> + <literal>(to prevent wraparound)</literal> or the
> + <literal>autovacuum_wraparound</literal> value in the
> + <structfield>started_by</structfield> column in the
> + <structname>pg_stat_progress_vacuum</structname> view), the
> autovacuum is
> + not automatically interrupted.
> </para>
> ...
> The new "or" statement does not follow the structure of the previous
> sentence. The earlier sentence uses the pattern "ends with (to prevent
> wraparound)." However, the "or" statement lacks a similar structure such
> as "ends with." A suggested rephrasing is: or if
> pg_stat_progress_vacuum.started_by is 'autovacuum_wraparound'

Agreed.

> 2.v9-0001
> ...
> + <listitem>
> + <para>
> + <literal>normal</literal>: The operation is performing a standard
> + vacuum. It is neither required to run in aggressive mode nor
> operating
> + in failsafe mode.
> + </para>
> + </listitem>
> ...
> Aggressive and failsafe are the other two modes explained later.
> Therefore, the sentence "It is neither required to run in aggressive
> mode nor operating in failsafe mode" is unnecessary and should be deleted.

I think we can leave it as is, since it's natural for an earlier
description to refer to something that comes later.

> 3.v9-0002
> ...
> + <para>
> + <literal>manual</literal>: The analyze was started by an explicit
> + <command>ANALYZE</command> or <command>VACUUM</command> with the
> + <option>ANALYZE</option> option.
> + </para>
> ...
>
> The current phrasing may lead to the misunderstanding that the "ANALYZE"
> option applies to both commands. It is recommended to revise it as: The
> analyze was started by an explicit <command>ANALYZE</command> command,
> or by <command>VACUUM (ANALYZE)</command>.

Fixed.

I've pushed both patches after incorporating the above two points.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com