Re: [PATCH] Identify LWLocks in tracepoints - Mailing list pgsql-hackers

From Andres Freund
Subject Re: [PATCH] Identify LWLocks in tracepoints
Date
Msg-id 20210413030651.s4yfuk6r6icbjr7y@alap3.anarazel.de
Whole thread Raw
In response to Re: [PATCH] Identify LWLocks in tracepoints  (Craig Ringer <craig.ringer@enterprisedb.com>)
Responses Re: [PATCH] Identify LWLocks in tracepoints  (Craig Ringer <craig.ringer@enterprisedb.com>)
Re: [PATCH] Identify LWLocks in tracepoints  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Hi,

On 2021-04-13 10:34:18 +0800, Craig Ringer wrote:
> > But it's near trivial to add that.
> 
> Really?

Yes.


> Each backend can have different tranche IDs (right?)

No, they have to be the same in each. Note how the tranche ID is part of
struct LWLock. Which is why LWLockNewTrancheId() has to acquire a lock
etc.


> But if I'm looking for performance issues caused by excessive LWLock
> contention or waits, LWLocks held too long, [...] or the like, it's
> something I want to capture across the whole postgres instance.

Sure.

Although I still don't really buy that static tracepoints are the best
way to measure this kind of thing, given the delay introducing them and
the cost of having them around. I think I pointed out
https://postgr.es/m/20200813004233.hdsdfvufqrbdwzgr%40alap3.anarazel.de
before.


> LWLock lock-ordering deadlocks

This seems unrelated to tracepoints to me.


> and there's no way to know what a given non-built-in tranche ID means
> for any given backend without accessing backend-specific in-memory
> state. Including for non-user-accessible backends like bgworkers and
> auxprocs, where it's not possible to just query the state from a view
> directly.

The only per-backend part is that some backends might not know the
tranche name for dynamically registered tranches where the
LWLockRegisterTranche() hasn't been executed in a backend. Which should
pretty much never be an aux process or such. And even for bgworkers it
seems like a pretty rare thing, because those need to be started by
something...

It might be worth proposing a shared hashtable with tranch names and
jut reserving enough space for ~hundred entries...

> And you can always build without `--enable-dtrace` and ... just not care.

Practically speaking, distributions enable it, which then incurs the
cost for everyone.



> Take a look at "sudo perf list".
> 
> 
>   sched:sched_kthread_work_execute_end               [Tracepoint event]
>   sched:sched_kthread_work_execute_start             [Tracepoint event]
>   ...
>   sched:sched_migrate_task                           [Tracepoint event]
>   ...
>   sched:sched_process_exec                           [Tracepoint event]
>   ...
>   sched:sched_process_fork                           [Tracepoint event]
>   ...
>   sched:sched_stat_iowait                            [Tracepoint event]
>   ...
>   sched:sched_stat_sleep                             [Tracepoint event]
>   sched:sched_stat_wait                              [Tracepoint event]
>   ...
>   sched:sched_switch                                 [Tracepoint event]
>   ...
>   sched:sched_wakeup                                 [Tracepoint event]
> 
> The kernel is packed with extremely useful trace events, and for very
> good reasons. Some on very hot paths.

IIRC those aren't really comparable - the kernel actually does modify
the executable code to replace the tracepoints with nops.


Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Mark Dilger
Date:
Subject: Re: pg_amcheck contrib application
Next
From: Fujii Masao
Date:
Subject: Re: TRUNCATE on foreign table