Re: the s_lock_stuck on perform_spin_delay

On Thu, Jan 18, 2024 at 1:30 PM Andy Fan <zhihuifan1213(at)163(dot)com> wrote:
> > You added an #include to dynahash.c despite making no other changes to
> > the file.
>
> That's mainly because I put the code into miscadmin.h and spin.h depends
> on miscadmin.h with MACROs.

That's not a good reason. Headers need to include headers on which
they depend; a .c file shouldn't be required to include one header
because it includes another.

> The LockBufHdr also used init_local_spin_delay / perform_spin_delay
> infrastruce and then it has the same issue like ${subject}, it is pretty
> like the code in s_lock; Based on my current knowledge, I think we
> should add the check there.

I'd like to hear from Andres, if possible. @Andres: Should these
sanity checks apply only to spin locks per se, or also to buffer
header locks?

> they were put into spin.h in v1 but later move to miscadmin.h at [1].
> [1]
> https://www.postgresql.org/message-id/CAEze2WggP-2Dhocmdhp-LxBzic%3DMXRgGA_tmv1G_9n-PDt2MQg%40mail.gmail.com

I'm not entirely sure what the right thing to do is here, and the
answer may depend on the previous question. But I disagree with
Matthias -- I don't think miscadmin.h can be the right answer
regardless.

> START_SPIN_LOCK need to be macro since it use __FILE__ and __LINE__ to
> note where the SpinLock is held. for others, just for consistent
> purpose. I think they can be changed to inline function, at least for
> VerifyNoSpinLocksHeld.

Good point about __FILE__ and __LINE__.

--
Robert Haas
EDB: http://www.enterprisedb.com