Re: [PATCH] Identify LWLocks in tracepoints

From: Andres Freund <andres(at)anarazel(dot)de>
To: Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, David Steele <david(at)pgmasters(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: [PATCH] Identify LWLocks in tracepoints
Date: 2021-04-13 03:06:51
Message-ID: 20210413030651.s4yfuk6r6icbjr7y@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2021-04-13 03:38:35 Re: TRUNCATE on foreign table
Previous Message Mark Dilger 2021-04-13 03:06:09 Re: pg_amcheck contrib application