Re: [PATCH] Add memory usage reporting to VACUUM VERBOSE

On Wed, Dec 24, 2025 at 1:07 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Dec 23, 2025, at 21:20, Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com> wrote:
> >
> > Hi Sawada-san and Chao-san,
> >
> > Thank you both for your continued reviews and feedback on this patch.
> >
> > > The patch mostly looks good to me. I've made some cosmetic changes to
> > > the comments (as well as the commit message) and attached the updated
> > > patch. Please review it.
> > Thank you. I have no objections.
> >
> >
> > > My last nitpick on v9:
> > >
> > > ```
> > > + appendStringInfo(&buf,
> > > + ngettext("memory usage: %.2f MB in total, with dead-item storage reset %d time (limit was %.2f MB)\n",
> > > + "memory usage: %.2f MB in total, with dead-item storage reset %d times (limit was %.2f MB)\n",
> > > ```
> > >
> > > Instead of “dead-item”, I would suggest “dead item” (without dash), because in the existing code use “dead item”, for example:
> > >
> > > ```
> > I've addressed your feedback.
> >
> > Attached is v10 with these changes incorporated.
> > I really appreciate reviewing this patch throughout the iterations.
> >
> > Regards,
> > Tatsuya Kawata
> > <v10-0001-Add-dead-items-memory-usage-to-VACUUM-VERBOSE-an.patch>
>
> V10 looks good to me.
>

Thank you for reviewing the patch!

After thinking about the verbose log message, I think we can improve
the verbose message to clarify the memory usage report more. For
example, if users get the message like:

memory usage: 102.77 MB in total, with dead item storage reset 520
times (limit was 0.12 MB)

They might confuse that vacuum used 102.77 MB memory in total in spite
of the limit being 0.12 MB. So how about rewording the message to?

memory usage: allocated 102.77 MB total, 520 dead item storage resets
(limit 0.12 MB each)

The rest of the changes look good to me.

Regards,

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