Re: Add per-backend lock statistics

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)