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
