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

On Tue, Dec 9, 2025 at 7:44 PM Tatsuya Kawata
<kawatatatsuya0913(at)gmail(dot)com> wrote:
>
> Hi all,
>
> Just a gentle ping on this patch.
> Please let me know if there are any additional comments
> or points I should address in the next revision.

Sorry for the late reply. I've reviewed the v7 patch and here are some comments:

+ /*
+ * Account for initial dead_items memory if nothing was collected.
+ */
+ if (vacrel->dead_items_info->num_items == 0 &&
vacrel->num_dead_items_resets == 0)
+ vacrel->total_dead_items_bytes +=
TidStoreMemoryUsage(vacrel->dead_items);

IIUC this line aims to add the initial data related to TidStore
including underlying radix tree and the bump context to the total
memory usage. I'm not sure why we do that only when no dead items are
collected. If we add these sizes, should we do that also when the
TidStore is reset due to being full and there are no dead items in the
subsequent blocks, no? I guess it would make more sense to consider
adding TidStoreMemoryUsage() also before cleaning up the TidStore.

---
+ appendStringInfo(&buf,
+ ngettext("memory usage: %.2f MB in total,
with dead-item storage reset %d time (memory allocated: %.2f MB)\n",
+ "memory usage: %.2f MB in total,
with dead-item storage reset %d times (memory allocated: %.2f MB)\n",
+ vacrel->num_dead_items_resets),
+ (double) vacrel->total_dead_items_bytes /
(1024 * 1024),
+ vacrel->num_dead_items_resets,
+ (double) dead_items_max_bytes / (1024 * 1024));

The memory usage report would be something like:

memory usage: 0.02 MB in total, with dead-item storage reset 0 times
(memory allocated: 64.00 MB)

I think it's not correct that we say "memory allocated: 64.00MB" in
this case because we don't actually allocate 64MB. Since we're using a
TidStore for dead items storage, we incrementally allocate its space
when needed.

Regards,

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