MIME-Version: 1.0
References: 
 <CAP53PkzdBK8VJ1fS4AZ481LgMN8f9mJiC39ZRHqkFUSYq6KWmg@mail.gmail.com>
 <h7zq426fhzt3oqkps53qxfwl3dlz3ifcwqmybuvxntlcaqwb33@m6huopxovgle>
 <CAP53Pkw1RpyAVsvJwUOMgGnVoQUibMk+EB+w-8WqY8kzsC4WjA@mail.gmail.com>
 <CAP53Pkz-BmkLFJ+sAcr8H1sBPtWp8WYGzorH9yw+W5Tn0yH0sg@mail.gmail.com>
 <mflexfbmbkmql5snr322ldu72dfnjr2dntxx3ippaz4tcso7vl@zmxxgiws3qux>
 <CAP53PkzZ3UotnRrrnXWAv=F4avRq9MQ8zU+bxoN9tpovEu6fGQ@mail.gmail.com>
 <k2uxwancbi5blb2duny3kbuz667m47hqc4jv7xstlx3yyzcd3a@fjqww6avalt3>
 <CAHut+Put94CTpjQsqOJHdHkgJ2ZXq+qVSfMEcmDKLiWLW-hPfA@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>
In-Reply-To: 
 <CAP53PkxAS-rpp0kcOjg--q5NVNnRC+0kh1+gB8gnRNqacTqF5A@mail.gmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Tue, 10 Mar 2026 08:12:56 +0000
Message-ID: 
 <CAN4CZFNuxMqnmtujzemZ1i7hzYkaLD+BWPNq8tFXoPWp5R4How@mail.gmail.com>
Subject: Re: Stack-based tracking of per-node WAL/buffer usage
To: Lukas Fittl <lukas@fittl.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>,
	Peter Smith <smithpb2250@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFNuxMqnmtujzemZ1i7hzYkaLD%2BBWPNq8tFXoPWp5R4How%40mail.gmail.com>
Precedence: bulk

> ResourceOwnerForgetInstrumentation directly follows the call to
> ExecFinalizeNodeInstrumentation in standard_ExecutorFinish, so I'm not
> sure which error case you're thinking of?

There are a few pallocs between them, so OOM is possible, even if
unlikely. I mainly mentioned this because even if unlikely it can
happen in theory, and the fix seems simple to me.

> I don't think that's a permanent leak, since it would be in the memory
> context of the caller, i.e. the per-query memory context

Yes, it's definitely not permanent, but could be bad with many tuples.

> and so
> doing the ResOwner dance for each tuple is probably not ideal.

These approaches are interesting, but also add complexity, so I'm
unsure which is better for this, the pfree calls add one line and
solve the main issue with the current code.

> Basically it should be this instead, I think,
> so we correctly call the table AM's table_index_fetch_tuple again if
> call_again gets set:

Right, this code will be better.

> I don't know if the extra allocations
> really matter, but I can see your point.

Yeah, probably doesn't matter that much, but the code also wasn't that
nice in that form. I didn't try to actually modify it, but by just
looking at it the grouped option seemed cleaner to me, and the output
should also be self-explanatory and logical to users.