Re: Add mode column to pg_stat_progress_vacuum - Mailing list pgsql-hackers

On 12/9/25 5:25 AM, Masahiko Sawada wrote:
> On Thu, Dec 4, 2025 at 8:30 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
>> Thank you for the review!
>>
>> On Thu, Dec 4, 2025 at 9:15 AM Masahiko Sawada <sawada.mshk@gmail.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