From: Tristan Partin <tristan@partin.io>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add per-backend lock statistics
Date: Fri, 05 Jun 2026 16:02:22 +0000
Message-ID: <DJ18Q72R3G27.34Z2PH78V1CTW@partin.io> (raw)
In-Reply-To: <aiKI2FSmZwyItCIz@bdtpg>
References: <aiAzEY+cMQb/W8yu@bdtpg>
<DJ0L4JF9W2J7.2B0DAKKQYA4DF@partin.io>
<aiKI2FSmZwyItCIz@bdtpg>
On Fri Jun 5, 2026 at 8:29 AM UTC, Bertrand Drouvot wrote:
> Hi,
>
> On Thu, Jun 04, 2026 at 09:32:39PM +0000, Tristan Partin wrote:
>> On Wed Jun 3, 2026 at 1:59 PM UTC, Bertrand Drouvot wrote:
>> > 0001: Refactor pg_stat_get_lock() to use a helper function
>>
>> > +static void
>> > +pg_stat_lock_build_tuples(ReturnSetInfo *rsinfo,
>> > + PgStat_LockEntry *lock_stats,
>> > + TimestampTz stat_reset_timestamp)
>>
>> I think that the alignment of the second and third arguments could be
>> off by one. They should line up with the capital R in ReturnSetInfo.
Probably just my editor being weird if it looks good to you!
> They look ok to me in the C file, what about you?
>
>> > - values[i] = TimestampTzGetDatum(lock_stats->stat_reset_timestamp);
>> > + if (stat_reset_timestamp != 0)
>> > + values[i] = TimestampTzGetDatum(stat_reset_timestamp);
>> > + else
>> > + nulls[i] = true;
>>
>> It's not super clear to me why this changed in the first patch.
>
> It's to make less "noise" in the second patch and keep the second patch focusing
> only on the "new feature". It's to ease to review but could be merged before
> being pushed would the commiter decides to do so.
Sounds good. To me it made the review a little more difficult, but
I understand the motivation.
>> > 0002: Add per-backend lock statistics
>>
>> > + Returns lock statistics about the backend with the specified
>> > + process ID. The output fields are exactly the same as the ones in the
>> > + <structname>pg_stat_lock</structname> view.
>>
>> It probably makes sense to link to pg_stat_lock here.
>
> Not sure as that would not be consistent with pg_stat_get_backend_io and
> pg_stat_get_backend_wal descriptions in monitoring.sgml.
Makes sense. Maybe we can update that in a future documentation update.
I'll go ahead and submit something in a separate thread.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tristan@partin.io, bertranddrouvot.pg@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Add per-backend lock statistics
In-Reply-To: <DJ18Q72R3G27.34Z2PH78V1CTW@partin.io>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox