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

From: 河田達也 <kawatatatsuya0913(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add memory usage reporting to VACUUM VERBOSE
Date: 2025-11-18 15:14:38
Message-ID: CAHza6qfrk-udc3_x_v9AzG3D6uTOGVS2W5vo7A0n9_7-kGHyXw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Sawada-san,

Thank you very much for your review and helpful comments on my previous
patch.
I have revised the patch following your suggestions:

・Report total memory usage rather than peak memory, which is more
meaningful for users.
・Removed the reset counter, since index scan counts already reflect memory
resets.
・Updated the code so that memory usage is recorded at the proper points,
avoiding unnecessary per-item checks.
・Removed the final peak memory check, as dead_items are already cleared at
that point.

All previous test cases have been re-run and verified to work correctly
with both serial and parallel VACUUM scenarios.
Please find the updated patch attached. I look forward to any further
feedback.

Best regards,
Tatsuya Kawata

2025年11月18日(火) 11:29 Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>:

> Hi,
>
> On Sun, Nov 16, 2025 at 8:01 AM 河田達也 <kawatatatsuya0913(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > I would like to propose a patch that adds memory usage reporting to
> > VACUUM VERBOSE output. This helps users understand how much memory
> > is being used for dead tuple tracking and whether memory limits are
> > being hit during vacuum operations.
>
> +1. The memory usage is already shown in the progress view but it
> would make sense to report it in the vacuum verbose log too.
>
> >
> > I have tested this patch with both serial and parallel VACUUM:
> > - Serial VACUUM with two maintenance_work_mem settings
> > - Parallel VACUUM with two maintenance_work_mem settings
> > - Cases with and without memory resets
> >
> > Test cases are included showing:
> > 1. Reset behavior with constrained memory (64KB)
> > 2. No reset behavior with ample memory (64MB)
> > 3. Both serial and parallel VACUUM scenarios
> >
> > I look forward to your feedback.
>
> Here are some review comments:
>
> + appendStringInfo(&buf,
> + _("memory
> usage: peak %.2f MB of %.2f MB allowed (%.1f%%), %d reset(s)\n"),
> + (double)
> vacrel->peak_dead_items_bytes / (1024.0 * 1024.0),
> + (double)
> vac_work_mem / 1024.0,
> + 100.0 *
> vacrel->peak_dead_items_bytes / (vac_work_mem * 1024.0),
> +
> vacrel->dead_items_resets);
>
> With the proposed patch, we report peak memory usage and reset count.
> Since we limit vacuum's memory usage to maintenance_work_mem, when
> performing one or more index vacuum operations, the peak memory usage
> typically equals maintenance_work_mem, with a reset count of 1 or
> higher. I wonder if it might be more useful to show the total memory
> used by vacuum. For example, if maintenance_work_mem is 512MB and
> vacuum uses 128MB during the second heap scan pass, the report would
> show a total memory usage of 640MB. I think probably the reset counter
> is not necessary as we already show the number of index scans.
>
> ---
> @@ -3590,6 +3629,10 @@ dead_items_add(LVRelState *vacrel, BlockNumber
> blkno, OffsetNumber *offsets,
> prog_val[0] = vacrel->dead_items_info->num_items;
> prog_val[1] = TidStoreMemoryUsage(vacrel->dead_items);
> pgstat_progress_update_multi_param(2, prog_index, prog_val);
> +
> + /* Track peak memory usage */
> + if (prog_val[1] > vacrel->peak_dead_items_bytes)
> + vacrel->peak_dead_items_bytes = prog_val[1];
> }
>
> The vacuum memory usage is monotonically increasing until it's reset.
> So we don't need to check the memory usage for every dead_items_add()
> call.
>
> ---
> + /*
> + * Capture final peak memory usage before cleanup.
> + * This is especially important for parallel vacuum where
> workers may have
> + * added items that weren't tracked in the leader's
> peak_dead_items_bytes.
> + */
> + if (vacrel->dead_items != NULL)
> + {
> + Size final_bytes =
> TidStoreMemoryUsage(vacrel->dead_items);
> +
> + if (final_bytes > vacrel->peak_dead_items_bytes)
> + vacrel->peak_dead_items_bytes = final_bytes;
> + }
>
> Please note that we call dead_items_reset() at the end of
> lazy_vacuum(). The dead items should already be empty at the above
> point.
>
> Regards,
>
> [1] https://commitfest.postgresql.org/57/
>
> --
> Masahiko Sawada
> Amazon Web Services: https://aws.amazon.com
>

Attachment Content-Type Size
test_memory_tracking.sql application/octet-stream 3.2 KB
v2-0001-Add-memory-usage-reporting-to-VACUUM-VERBOSE.patch application/octet-stream 3.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sugamoto Shinya 2025-11-18 15:20:34 Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions
Previous Message David Geier 2025-11-18 15:07:04 Re: Performance issues with parallelism and LIMIT