public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: Greg Sabino Mullane <[email protected]>
Cc: [email protected]
Subject: Re: Adding locks statistics
Date: Tue, 24 Mar 2026 16:02:55 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<isi5wszczhusgjetxwe6khxa5mbm5jms3msmbhwl73jktxw7ic@3mympoumua76>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Sat, Mar 21, 2026 at 02:55:33PM +0900, Michael Paquier wrote:
> On Thu, Mar 19, 2026 at 12:25:41PM +0000, Bertrand Drouvot wrote:
>> I did not check if there are any other files that could benefit of using
>> locktag.h instead of lock.h but that's something I'll do and open a dedicated
>> if any (once locktag.h is in the tree).
> 
> I have checked after that, and did not spot an area (except your patch
> of course).  And applied this part.

The main patch has some churn in proc.c and lock.c, moving some code
blocks while the main focus of the patch is to add the two pgstat()
calls to report some data, so I have moved this part into its own
commit, and applied it.  One thing to not in lock.c: we will calculate
the time difference of the wait even if log_lock_waits is disabled,
without the lock stats part.  As this GUC is enabled by default, it
does not matter much in practice, I guess, and it matters even less
with the main patch merged.

The implementation of the main patch is close enough to pgstat_io.c
that your logic is a no-brainer (timestamp protected by the first
LWLock, each field incremented depending on locktags, etc.).  Instead
of a boolean flag tracking if some stats have been set, we could also
have used an approach like the checkpointer stats with is_all_zeros on
the pending data, perhaps?  At the end, with two incrementation
routines, I've let the code as-is.

Another thing I am not completely sure is if the sleep time of the
isolation tests is long enough.  I have tested things with
CLOBBER_CACHE_ALWAYS to make the setup more sensitive to timings but
could not get it to fail.  We'll know soon enough if the buildfarm
complains.

After a few more tweaks here and there (code, comments, some
beautification), done.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

view thread (46+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Adding locks statistics
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox