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.