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

Hi Tatsuya-san,

I just reviewed and tested v6, and got a major concern and a few small comments.

> On Nov 27, 2025, at 22:59, 河田達也 <kawatatatsuya0913@gmail.com> wrote:
> 
> Hi Sawada-san,
> I rebased and applied the changes you suggested.
> The updated v6 is attached.
> Best regards,
> Tatsuya Kawata
> <v6-0001-Add-memory-usage-reporting-to-VACUUM-VERBOSE.patch>

1. Major concern: In this patch, you only increase vacrel->total_dead_items_bytes in dead_items_reset(), however, dead_items_reset() is not always called, that way I often see usage is 0. We should also increate the counter in heap_vacuum_rel() where you now get dead_items_max_bytes. My dirty fix is like below:

```
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index ef2c72edf5d..635e3cf8346 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -862,6 +862,8 @@ heap_vacuum_rel(Relation rel, const VacuumParams params,
         */
        dead_items_max_bytes = vacrel->dead_items_info->max_bytes;

+       vacrel->total_dead_items_bytes += TidStoreMemoryUsage(vacrel->dead_items);
+
        /*
         * Free resources managed by dead_items_alloc.  This ends parallel mode in
         * passing when necessary.
```

I verified the fix with a simple procedure:
```
drop table if exists vacuum_test;
create table vacuum_test (id int);
insert into vacuum_test select generate_series(1, 100000);
delete from vacuum_test WHERE id % 2 = 0;
vacuum (verbose) vacuum_test;
```

Without my fix, memory usage was reported as 0:
```
memory usage: 0.00 MB in total, with dead-item storage reset 0 times (limit was 64.00 MB)
```

And with my fix, memory usage is greater than 0:
```
memory usage: 0.02 MB in total, with dead-item storage reset 0 times (limit was 64.00 MB)
```

2
```
+	 * Save dead items max_bytes before cleanup for reporting the memory usage
+	 * as the dead_items_info is freed in parallel vacuum cases during
+	 * cleanup.
+	 */
+	dead_items_max_bytes = vacrel->dead_items_info->max_bytes;
```

The comment says "the dead_items_info is freed in parallel vacuum”, should we check if vacrel->dead_items_info != NULL before deferencing max_bytes?

3
```
+			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",

```

I just feel “limit was xxx MB” is still not clear enough. How about be explicit, like:

   Memory usage: 0.2 MB in total, memory allocated across 0 dead-item storage resets in total: 64.00 MB

Or 

   Memory usage: 0.2 MB in total, with dead-item storage reset %d time, total allocated: 64.00 MB


Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/