Re: Add mode column to pg_stat_progress_vacuum

From: Yu Wang <wangyu_runtime(at)163(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, Robert Treat <rob(at)xzilla(dot)net>, maciek(at)sakrejda(dot)org, Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add mode column to pg_stat_progress_vacuum
Date: 2025-12-09 08:21:57
Message-ID: 7bbb50c8-f865-4e75-bf8b-a0e54fd564c0@163.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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'

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.

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>.

regards
--
Yu Wang

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2025-12-09 08:43:25 Re: commented out code
Previous Message Kirill Reshke 2025-12-09 07:30:51 Re: citext_1.out, citext.out confusing comment