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