Logging in LockBufferForCleanup()

Hi,

I just noticed somewhat new code in LockBufferForCleanup(), added in

commit 39b03690b529935a3c33024ee68f08e2d347cf4f
Author: Fujii Masao <fujii(at)postgresql(dot)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(at)postgresql(dot)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