From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, Jaime Casanova <jaime.casanova@2ndQuadrant.com>
Subject: Re: LogwrtResult contended spinlock
Date: Fri, 29 Jan 2021 12:40:18 -0300
Message-ID: <20210129154018.GA7723@alvherre.pgsql> (raw)
In-Reply-To: <CE5FB7B4-BD5B-40C1-A915-0AE770812C3D@anarazel.de>
On 2020-Aug-31, Andres Freund wrote:
> Wouldn't the better fix here be to allow reading of individual members without a lock? E.g. by wrapping each in a 64bit atomic.
So I've been playing with this and I'm annoyed about having two
datatypes to represent Write/Flush positions:
typedef struct XLogwrtRqst
{
XLogRecPtr Write; /* last byte + 1 to write out */
XLogRecPtr Flush; /* last byte + 1 to flush */
} XLogwrtRqst;
typedef struct XLogwrtResult
{
XLogRecPtr Write; /* last byte + 1 written out */
XLogRecPtr Flush; /* last byte + 1 flushed */
} XLogwrtResult;
Don't they look, um, quite similar? I am strongly tempted to remove
that distinction, since it seems quite pointless, and introduce a
different one:
typedef struct XLogwrtAtomic
{
pg_atomic_uint64 Write; /* last byte + 1 of write position */
pg_atomic_uint64 Flush; /* last byte + 1 of flush position */
} XLogwrtAtomic;
this one, with atomics, would be used for the XLogCtl struct members
LogwrtRqst and LogwrtResult, and are always accessed using atomic ops.
On the other hand we would have
typedef struct XLogwrt
{
XLogRecPtr Write; /* last byte + 1 of write position */
XLogRecPtr Flush; /* last byte + 1 of flush position */
} XLogwrt;
to be used for the local-memory-only LogwrtResult, using normal
assignment.
Now, I do wonder if there's a point in keeping LogwrtResult as a local
variable at all; maybe since the ones in shared memory are going to use
unlocked access, we don't need it anymore? I'd prefer to defer that
decision to after this patch is done, since ISTM that it'd merit more
careful benchmarking.
Thoughts?
--
Álvaro Herrera Valdivia, Chile
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: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, andres@anarazel.de, jaime.casanova@2ndQuadrant.com
Subject: Re: LogwrtResult contended spinlock
In-Reply-To: <20210129154018.GA7723@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox