Re: [PATCH] Identify LWLocks in tracepoints

On 10.03.21 06:38, Craig Ringer wrote:
> On Wed, 3 Mar 2021 at 20:50, David Steele <david@pgmasters.net 
> <mailto:david@pgmasters.net>> wrote:
> 
>     On 1/22/21 6:02 AM, Peter Eisentraut wrote:
> 
>     This patch set no longer applies:
>     http://cfbot.cputube.org/patch_32_2927.log
>     <http://cfbot.cputube.org/patch_32_2927.log>.
> 
>     Can we get a rebase? Also marked Waiting on Author.
> 
> 
> Rebased as requested.
> 
> I'm still interested in whether Andres will be able to do anything about 
> identifying LWLocks in a cross-backend manner. But this work doesn't 
> really depend on that; it'd benefit from it, but would be easily adapted 
> to it later if needed.

First, a problem:  0002 doesn't build on macOS, because uint64 has been 
used in the probe definitions.  That needs to be handled like the other 
nonnative types in that file.

All the probe changes and additions should be accompanied by 
documentation changes.

The probes used to have an argument to identify the lock, which was 
removed by 3761fe3c20bb040b15f0e8da58d824631da00caa.  The 0001 patch is 
essentially trying to reinstate that, which seems sensible.  Perhaps we 
should also use the argument order that used to be there.  It used to be

probe lwlock__acquire(const char *, int, LWLockMode);

and now it would be

probe lwlock__acquire(const char *, LWLockMode, LWLock*, int);

Also, do we need both the tranche name and the tranche id?  Or maybe we 
don't need the name, or can record it differently, which might also 
address your other concern that it's too expensive to compute.  In any 
case, I think an argument order like

probe lwlock__acquite(const char *, int, LWLock*, LWLockMode);

would make more sense.

In 0004, you add a probe to record the application_name setting?  Would 
there be any value in making that a generic probe that can record any 
GUC change?