public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alvaro Herrera <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Lockless exit path for ReplicationOriginExitCleanup
Date: Wed, 22 Nov 2023 10:36:53 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACX+YaeRU5xJqR4C7kLsTO_F7DBRNF8WgeHvJZcKtNuK_A@mail.gmail.com>

Hello,

On 2023-Nov-22, Bharath Rupireddy wrote:

> While looking at the use of session_replication_state, I noticed that
> ReplicationOriginLock is acquired in ReplicationOriginExitCleanup()
> even if session_replication_state is reset to NULL by
> replorigin_session_reset(). Why can't there be a lockless exit path
> something like [1] similar to
> replorigin_session_reset() which checks session_replication_state ==
> NULL without a lock?

I suppose we can do this on consistency grounds -- I'm pretty sure you'd
have a really hard time proving that this makes a performance difference --
but this patch is incomplete: just two lines below, we're still testing
session_replication_state for nullness, which would now be dead code.
Please repair.


The comment on session_replication_state is confusing also:

/*
 * Backend-local, cached element from ReplicationState for use in a backend
 * replaying remote commits, so we don't have to search ReplicationState for
 * the backends current RepOriginId.
 */

My problem with it is that this is not a "cached element", but instead a
"cached pointer to [shared memory]".  This is what makes testing that
pointer for null-ness doable, but access to each member therein
requiring lwlock acquisition.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Once again, thank you and all of the developers for your hard work on
PostgreSQL.  This is by far the most pleasant management experience of
any database I've worked on."                             (Dan Harris)
http://archives.postgresql.org/pgsql-performance/2006-04/msg00247.php






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: Lockless exit path for ReplicationOriginExitCleanup
  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