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

Hi Zsolt,

On Tue, Mar 24, 2026 at 3:59 PM Zsolt Parragi <zsolt.parragi@percona.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