Re: Stack-based tracking of per-node WAL/buffer usage

From: Lukas Fittl <lukas(at)fittl(dot)com>
To: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tomas(at)vondra(dot)me>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Smith <smithpb2250(at)gmail(dot)com>
Subject: Re: Stack-based tracking of per-node WAL/buffer usage
Date: 2026-03-25 05:34:29
Message-ID: CAP53PkwTxo5R8_x76PmixXBV2YBGGf+7miRP28Asr7u-CLhQfA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Zsolt,

On Tue, Mar 24, 2026 at 3:59 PM Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
> I like the new approach, but doesn't `EXPLAIN (BUFFERS)` leak some
> memory because the resource owner isn't registered on that path? It
> seems to be visible with pg_log_backend_memory_contexts.

Ah, yes, good catch! Its nice how the separate memory context makes
this very clear now.

I think this was actually an existing problem that only surfaced now.
The issue is that EXPLAIN (BUFFERS) will allocate the per-query
instrumentation, but not actually use it, since the buffer tracking is
only relevant for planning, which has its own instrumentation. I can
fix this locally by adding the following to FreeExecutorState:

/*
* Make sure the instrumentation context gets freed. This usually gets
* re-parented under the per-query context in InstrQueryStopFinalize, but
* that won't happen during EXPLAIN (BUFFERS) since ExecutorFinish never
* gets called, so we would otherwise leak it in TopMemoryContext.
*/
if (estate->es_instrument && estate->es_instrument->instr.need_stack)
MemoryContextDelete(estate->es_instrument->instr_cxt);

I'll include this in the next revision unless I come up with a better
idea. FWIW, I also considered just not setting INSTRUMENT_BUFFERS in
ExplainOnePlan unless ANALYZE is active, but I think there might be
other cases where that doesn't work as expected, so I think the
explicit delete is better.

>
> #define INSTR_BUFUSAGE_ADD(fld,val) do { \
> - pgBufferUsage.fld += val; \
> + instr_stack.current->bufusage.fld += val; \
>
> #define INSTR_WALUSAGE_ADD(fld,val) do { \
> pgWalUsage.fld += val; \
> + instr_stack.current->walusage.fld += val; \
>
> Nitpick, but these could use += (val)

Ack, makes sense - I'll adjust in the next revision.

I'll give it a day or so for further feedback before posting the next update.

Thanks,
Lukas

--
Lukas Fittl

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2026-03-25 05:38:58 Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?
Previous Message Michael Paquier 2026-03-25 05:25:01 Re: Adding locks statistics