On Tue, Dec 9, 2025 at 12:22 AM Yu Wang <[email protected]> wrote: > > On 12/9/25 5:25 AM, Masahiko Sawada wrote: > > On Thu, Dec 4, 2025 at 8:30 PM Shinya Kato <[email protected]> wrote: > >> Thank you for the review! > >> > >> On Thu, Dec 4, 2025 at 9:15 AM Masahiko Sawada <[email protected]> > >> 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
