Re: [PATCH] Identify LWLocks in tracepoints

On 19.03.21 21:06, Peter Eisentraut wrote:
> On 18.03.21 07:34, Craig Ringer wrote:
>>     In patch 0001, why was the TRACE_POSTGRESQL_LWLOCK_RELEASE() call
>>     moved?
>>        Is there some correctness issue?  If so, we should explain that
>> (at
>>     least in the commit message, or as a separate patch).
>>
>>
>> If you want I can split it out, or drop that change. I thought it was
>> sufficiently inconsequential, but you're right to check.
>>
>> The current tracepoint TRACE_POSTGRESQL_LWLOCK_RELEASE really means
>> "releaseD". It's appropriate to emit this as soon as the lock could be
>> acquired by anything else. By deferring it until we'd processed the
>> waitlist and woken other backends the window during which the lock was
>> reported as "held" was longer than it truly was, and it was easy to
>> see one backend acquire the lock while another still appeared to hold it.
>
> From the archeology department: The TRACE_POSTGRESQL_LWLOCK_RELEASE
> probe was in the right place until PG 9.4, but was then moved by
> ab5194e6f617a9a9e7aadb3dd1cee948a42d0755, which was a major rewrite, so
> it seems the move might have been accidental.  The documentation
> specifically states that the probe is triggered before waiters are woken
> up, which it specifically does not do at the moment.  So this looks like
> a straight bug fix to me.

committed a fix for that