Hi,
I just noticed somewhat new code in LockBufferForCleanup(), added in
commit 39b03690b529935a3c33024ee68f08e2d347cf4f
Author: Fujii Masao <fujii@postgresql.org>
Date: 2021-01-13 22:59:17 +0900
Log long wait time on recovery conflict when it's resolved.
commit 64638ccba3a659d8b8a3a4bc5b47307685a64a8a
Author: Fujii Masao <fujii@postgresql.org>
Date: 2020-03-30 17:35:03 +0900
Report waiting via PS while recovery is waiting for buffer pin in hot standby.
After those commit LockBufferForCleanup() contains code doing memory
allocations, elogs. That doesn't strike me as a good idea:
Previously the code looked somewhat safe to use in critical section like
blocks (although whether it'd be good idea to use in one is a different
question), but not after. Even if not used in a critical section, adding new
failure conditions to low-level code that's holding LWLocks etc. doesn't seem
like a good idea.
Secondly, the way it's done seems like a significant laying violation. Before
the HS related code in LockBufferForCleanup() was encapsulated in a few calls
to routines dedicated to dealing with that. Now it's all over
LockBufferForCleanup().
It also just increases the overhead of LockBuffer(). Adding palloc(), copying
of process title, GetCurrentTimestamp() to a low level routine like this isn't
free - even if it's mostly in the contended paths.
Greetings,
Andres Freund