public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: [email protected]
To: Masahiko Sawada <[email protected]>
To: Fujii Masao <[email protected]>
Subject: Logging in LockBufferForCleanup()
Date: Wed, 9 Feb 2022 18:22:05 -0800
Message-ID: <[email protected]> (raw)

Hi,

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

commit 39b03690b529935a3c33024ee68f08e2d347cf4f
Author: Fujii Masao <[email protected]>
Date:   2021-01-13 22:59:17 +0900
 
    Log long wait time on recovery conflict when it's resolved.

commit 64638ccba3a659d8b8a3a4bc5b47307685a64a8a
Author: Fujii Masao <[email protected]>
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

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Logging in LockBufferForCleanup()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox