public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jeff Davis <[email protected]>
To: Bharath Rupireddy <[email protected]>
To: Andres Freund <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: Nathan Bossart <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Subject: Re: Improve WALRead() to suck data directly from WAL buffers when possible
Date: Thu, 07 Dec 2023 16:34:57 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACVfFMfqD5oLzZSQQZWfXiJqd-NdX0_317veP6FuB31QWA@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CALj2ACU2DGH-BB_187N7gfVkLdxfJ+doVTcteS0ugLn_c8mLOw@mail.gmail.com>
<CALj2ACVgVgA5BSSrEYO2eTMEGB=QUbcYosYm3vZ3R2=GPB6tNw@mail.gmail.com>
<[email protected]>
<CALj2ACVjzHgQvONnTLffyBJRACPwhsf-cYG4TQ2KHKrFfFop-w@mail.gmail.com>
<[email protected]>
<CALj2ACV123Ke6wn05QCnCgJhFpcarGCv7sKM1p39t+qpUNHUgQ@mail.gmail.com>
<[email protected]>
<CALj2ACUSydTHPcucftm4XdiJ4osH8zeNENyADd-zfDzEMf6akA@mail.gmail.com>
<[email protected]>
<CALj2ACXo-_boM3h-PJuvfuvFzP1nv2wcZBoZN_YbRbQBGQYAUg@mail.gmail.com>
<CALj2ACVfFMfqD5oLzZSQQZWfXiJqd-NdX0_317veP6FuB31QWA@mail.gmail.com>
On Thu, 2023-12-07 at 15:59 +0530, Bharath Rupireddy wrote:
> In the attached v17 patch
0001 could impact performance could be impacted in a few ways:
* There's one additional write barrier inside
AdvanceXLInsertBuffer()
* AdvanceXLInsertBuffer() already holds WALBufMappingLock, so
the atomic access inside of it is somewhat redundant
* On some platforms, the XLogCtlData structure size will change
The patch has been out for a while and nobody seems concerned about
those things, and they look fine to me, so I assume these are not real
problems. I just wanted to highlight them.
Also, the description and the comments seem off. The patch does two
things: (a) make it possible to read a page without a lock, which means
we need to mark with InvalidXLogRecPtr while it's being initialized;
and (b) use 64-bit atomics to make it safer (or at least more
readable).
(a) feels like the most important thing, and it's a hard requirement
for the rest of the work, right?
(b) seems like an implementation choice, and I agree with it on
readability grounds.
Also:
+ * But it means that when we do this
+ * unlocked read, we might see a value that appears to be ahead of
the
+ * page we're looking for. Don't PANIC on that, until we've verified
the
+ * value while holding the lock.
Is that still true even without a torn read?
The code for 0001 itself looks good. These are minor concerns and I am
inclined to commit something like it fairly soon.
Regards,
Jeff Davis
view thread (26+ messages) latest in thread
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], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Improve WALRead() to suck data directly from WAL buffers when possible
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