MIME-Version: 1.0
References: 
 <CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com>
 <CAP53PkyOvXC7pWAiamvWth_JNeb=isrxX+PJT0pw_Hw5Czzf+Q@mail.gmail.com>
 <CAP53PkzGbyeJMLDAcvMRgzXPXYsYXZr3SBg0UwhfkYjqu8oK_g@mail.gmail.com>
 <CAP53PkxrmpECzVFpeeEEHDGe6u625s+YkmVv5-gw3L_NDSfbiA@mail.gmail.com>
 <CAN4CZFMon7XcTv+PKXB_ANUJ86k9VmJnx7BoW2q1m5Z2QQVZQw@mail.gmail.com>
 <CAP53PkxAS-rpp0kcOjg--q5NVNnRC+0kh1+gB8gnRNqacTqF5A@mail.gmail.com>
 <CAN4CZFNuxMqnmtujzemZ1i7hzYkaLD+BWPNq8tFXoPWp5R4How@mail.gmail.com>
 <CAP53PkxFP7i7-wy98ZmEJ11edYq-RrPvJoa4kzGhBBjERA4Nyw@mail.gmail.com>
 <06086cb4-881b-4f5a-96af-f275220ff52d@vondra.me>
 <CAN4CZFOdYhRfecTJjJZCwHtS-gzpSJ16YLQ7Wh0bXrg0=8keOw@mail.gmail.com>
 <CAP53PkxfVnQpcjrwegamPKdyy3RExXXeXNQW1ZWWtr=JtNsLNQ@mail.gmail.com>
 <CAN4CZFMk10YCixVTnt+uBaUH_BH1R=Er-PnPs2YDBhFjuE3K-Q@mail.gmail.com>
 <CAP53Pkwdg7Pmf+OqUBu4_SKgxEH3oosSrxxMc4v=mCqMUB049A@mail.gmail.com>
 <CAP53PkyEPa=aFZrd0crLgRJm1KGS0ykJ+1i2CwDfDNFKpCc+XQ@mail.gmail.com>
 <da7ad516-37d1-47e5-afae-26a7ef7b60f8@iki.fi>
 <CAN4CZFOUTHdXuEAwrST8ueDxGJRe69zVgeVAY0osTjVkRZi_Lw@mail.gmail.com>
In-Reply-To: 
 <CAN4CZFOUTHdXuEAwrST8ueDxGJRe69zVgeVAY0osTjVkRZi_Lw@mail.gmail.com>
From: Lukas Fittl <lukas@fittl.com>
Date: Mon, 23 Mar 2026 13:03:21 -0700
Message-ID: 
 <CAP53Pkx+HZ2OSGDHmNgSDisXdjGp4KFygaX+GaQcFEuXHRXg=g@mail.gmail.com>
Subject: Re: Stack-based tracking of per-node WAL/buffer usage
To: Zsolt Parragi <zsolt.parragi@percona.com>,
 Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tomas Vondra <tomas@vondra.me>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>,
	Peter Smith <smithpb2250@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAP53Pkx%2BHZ2OSGDHmNgSDisXdjGp4KFygaX%2BGaQcFEuXHRXg%3Dg%40mail.gmail.com>
Precedence: bulk

On Mon, Mar 23, 2026 at 12:07=E2=80=AFPM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:
>
> > I'm looking at this finalize at resowner part of this patch, and this
> > maybe a stupid question, but:
> >
> > Why does the instrumentation need to be "finalized" on abort? If you ru=
n
> > EXPLAIN ANALYZE and the query aborts, you don't get to see the stats an=
yway.
>
> The pg_session_buffer_usage in 0009 makes the information available, I
> was able to see the issue with failing triggers with that. Even if
> that part doesn't get committed in the end, a 3rd party extension
> could still implement the same thing, and notice the missing
> statistics. (And maybe it is useful to see some statistics about
> failing queries?)

Right, there are basically two reasons we need the resource owner
logic, or something equivalent:

1) To ensure the active stack entry gets reset correctly to where it
was before, so that we don't corrupt it after an abort (if that's not
done, we'll fail during regular regression tests)

2) To accumulate statistics to parent stack entries that did not
participate in the abort

As Zsolt noted, (2) matters for the pg_session_buffer_usage module,
which is mainly intended to ensure that the logic we're adding keeps a
top-level instrumentation available that includes the activity of
aborted transactions/queries, since we're removing pgBufferUsage. It
also matters for some edge cases in-tree today, e.g. procedures being
tracked in pg_stat_statements. And I do see us being interested in
tracking failed query activities in the future as Zsolt noted.

---

FWIW, on the topic of resource owners and allocations, I've done a
test over the weekend, and here is a question:

It seems we could switch the Instrumentation allocations we're doing
when inside a portal to PortalContext, and CurrentMemoryContext when
outside a portal - instead of allocating in
TopMemoryContext/TopTransactionContext. That works in practice,
because resource owner cleanup happens before PortalContext cleanup,
and simplifies the code a bit since we can skip copying into the
current memory context (because the caller wants to be able to use the
result after the finalize call). And if we leak we'd only leak until
PortalContext gets cleaned up, instead of TopMemoryContext.

To expand on that, in the previously posted v9 we have the following
allocations:

A) InstrStackState allocated under TopMemoryContext (long-lived, never free=
d)
B) QueryInstrumentation allocated under TopMemoryContext (short-lived
during query execution, explicitly freed up on abort or finalize call)
C) NodeInstrumentation allocated under TopTransactionContext
(short-lived during query execution, explicitly freed up on abort or
finalize call)
D) In other use cases, e.g. ANALYZE command that logs buffer usage,
QueryInstrumentation allocated under TopMemoryContext (short-lived
during command execution, explicitly freed up on abort or finalize
call)

And we could switch it instead to:

A) InstrStackState allocated under TopMemoryContext (long-lived, never free=
d)
B) QueryInstrumentation allocated under PortalContext (short-lived
during query execution, *automatically* freed up on abort, manually on
ExecutorEnd to avoid waiting for holdable cursors to free
PortalContext)
C) NodeInstrumentation allocated under PortalContext (short-lived
during query execution, *automatically* freed up on abort, manually on
ExecutorEnd to avoid waiting for holdable cursors to free
PortalContext)
D) In other use cases, e.g. ANALYZE command that logs buffer usage,
QueryInstrumentation allocated under CurrentMemoryContext (short-lived
during command execution, *automatically* freed up on abort and
success case)

However, this goes against the principle noted by Heikki over in [0]
that ResOwners should use TopMemoryContext to avoid relying on the
ordering of clean up operations.

Thoughts?

Thanks,
Lukas

[0]: https://www.postgresql.org/message-id/flat/a3197b31-f40d-4164-872d-906=
d8e9b374a%40iki.fi#526984c1be0662a7526bcfe0b358564b


--
Lukas Fittl