On Wed, Apr 24, 2024 at 09:46:20PM +0530, Bharath Rupireddy wrote:
> Thanks. I started a new thread
> https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com.
Cool, thanks.
--
Michael
signature.asc
Description: PGP signature
On Thu, Apr 11, 2024 at 6:31 AM Michael Paquier wrote:
>
> On Tue, Apr 09, 2024 at 09:33:49AM +0300, Andrey M. Borodin wrote:
> > As far as I understand CF entry [0] is committed? I understand that
> > there are some open followups, but I just want to determine correct
> > CF item status...
>
>
On Tue, Apr 09, 2024 at 09:33:49AM +0300, Andrey M. Borodin wrote:
> As far as I understand CF entry [0] is committed? I understand that
> there are some open followups, but I just want to determine correct
> CF item status...
So much work has happened on this thread with things that has been
> On 8 Apr 2024, at 08:17, Bharath Rupireddy
> wrote:
Hi Bharath!
As far as I understand CF entry [0] is committed? I understand that there are
some open followups, but I just want to determine correct CF item status...
Thanks!
Best regards, Andrey Borodin.
[0]
On Thu, Mar 21, 2024 at 11:33 PM Bharath Rupireddy
wrote:
>
> Please find the v26 patches after rebasing.
Commit f3ff7bf83b added a check in WALReadFromBuffers to ensure the
requested WAL is not past the WAL that's copied to WAL buffers. So,
I've dropped v26-0001 patch.
I've attached v27
On Tue, Feb 20, 2024 at 11:40 AM Bharath Rupireddy
wrote:
>
> Ran pgperltidy on the new TAP test file added. Please see the attached
> v25 patch set.
Please find the v26 patches after rebasing.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services:
On Sat, Feb 17, 2024 at 10:27 AM Bharath Rupireddy
wrote:
>
> On Fri, Feb 16, 2024 at 11:01 PM Jeff Davis wrote:
> >
> > > Here, I'm with v23 patch set:
> >
> > Thank you, I'll look at these.
>
> Thanks. Here's the v24 patch set after rebasing.
Ran pgperltidy on the new TAP test file added.
On Fri, Feb 16, 2024 at 11:01 PM Jeff Davis wrote:
>
> > Here, I'm with v23 patch set:
>
> Thank you, I'll look at these.
Thanks. Here's the v24 patch set after rebasing.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From
On Fri, 2024-02-16 at 13:08 +0530, Bharath Rupireddy wrote:
> I'd suggest we strike a balance here - error out in assert builds if
> startptr+count is past the current insert position and trust the
> callers for production builds.
It's not reasonable to have divergent behavior between
On Wed, Feb 14, 2024 at 6:59 AM Jeff Davis wrote:
>
> Attached 2 patches.
>
> Per Andres's suggestion, 0001 adds an:
> Assert(startptr + count <= LogwrtResult.Write)
>
> Though if we want to allow the caller (e.g. in an extension) to
> determine the valid range, perhaps using
Hi,
On 2024-02-12 17:33:24 -0800, Jeff Davis wrote:
> On Mon, 2024-02-12 at 15:36 -0800, Andres Freund wrote:
> >
> > It doesn't really seem like a necessary, or even particularly useful,
> > part. You couldn't just call WALRead() for that, since the caller
> > would need
> > to know the range up
On Tue, 2024-02-13 at 22:47 +0530, Bharath Rupireddy wrote:
> c) If planning to replicate unflushed data, then ensure all the
> WALRead() callers wait until startptr+count is past the current
> insert
> position with WaitXLogInsertionsToFinish().
WALRead() can't read past the Write pointer, so
Attached 2 patches.
Per Andres's suggestion, 0001 adds an:
Assert(startptr + count <= LogwrtResult.Write)
Though if we want to allow the caller (e.g. in an extension) to
determine the valid range, perhaps using WaitXLogInsertionsToFinish(),
then the check is wrong. Maybe we should just get rid
On Tue, Feb 13, 2024 at 5:06 AM Andres Freund wrote:
>
> I doubt there's a sane way to use WALRead() without *first* ensuring that the
> range of data is valid. I think we're better of moving that responsibility
> explicitly to the caller and adding an assertion verifying that.
>
> It doesn't
On Tue, Feb 13, 2024 at 1:03 AM Jeff Davis wrote:
>
> For 0004, we need to resolve why callers are using XLOG_BLCKSZ and we
> can fix that independently, as discussed here:
>
> https://www.postgresql.org/message-id/CALj2ACV=C1GZT9XQRm4iN1NV1T=hla_hsgwnx2y5-g+mswd...@mail.gmail.com
Thanks. I
On Mon, 2024-02-12 at 15:36 -0800, Andres Freund wrote:
>
> It doesn't really seem like a necessary, or even particularly useful,
> part. You couldn't just call WALRead() for that, since the caller
> would need
> to know the range up to which WAL is valid but not yet flushed as
> well. Thus
> the
Hi,
On 2024-02-12 15:56:19 -0800, Jeff Davis wrote:
> On Mon, 2024-02-12 at 11:33 -0800, Jeff Davis wrote:
> > For 0002 & 0003, I'd like more clarity on how they will actually be
> > used by an extension.
>
> In patch 0002, I'm concerned about calling
> WaitXLogInsertionsToFinish(). It loops
On Mon, 2024-02-12 at 11:33 -0800, Jeff Davis wrote:
> For 0002 & 0003, I'd like more clarity on how they will actually be
> used by an extension.
In patch 0002, I'm concerned about calling
WaitXLogInsertionsToFinish(). It loops through all the locks, but
doesn't have any early return path or
Hi,
On 2024-02-12 12:46:00 -0800, Jeff Davis wrote:
> On Mon, 2024-02-12 at 12:18 -0800, Andres Freund wrote:
> > + upto = Min(startptr + count, LogwrtResult.Write);
> > + nbytes = upto - startptr;
> >
> > Shouldn't it pretty much be a bug to ever encounter this?
>
> In the current code
On Mon, 2024-02-12 at 12:18 -0800, Andres Freund wrote:
> + upto = Min(startptr + count, LogwrtResult.Write);
> + nbytes = upto - startptr;
>
> Shouldn't it pretty much be a bug to ever encounter this?
In the current code it's impossible, though Bharath hinted at an
extension which could
Hi,
On 2024-02-12 11:33:24 -0800, Jeff Davis wrote:
> On Wed, 2024-01-31 at 14:30 +0530, Bharath Rupireddy wrote:
> > Please see the attached v22 patch set.
>
> Committed 0001.
Yay, I think this is very cool. There are plenty other improvements than can
be based on this...
One thing I'm a bit
On Wed, 2024-01-31 at 14:30 +0530, Bharath Rupireddy wrote:
> Please see the attached v22 patch set.
Committed 0001.
For 0002 & 0003, I'd like more clarity on how they will actually be
used by an extension.
For 0004, we need to resolve why callers are using XLOG_BLCKSZ and we
can fix that
On Wed, Jan 31, 2024 at 3:01 PM Alvaro Herrera wrote:
>
> Looking at 0003, where an XXX comment is added about taking a spinlock
> to read LogwrtResult, I suspect the answer is probably not, because it
> is likely to slow down the other uses of LogwrtResult.
We avoided keeping LogwrtResult
Looking at 0003, where an XXX comment is added about taking a spinlock
to read LogwrtResult, I suspect the answer is probably not, because it
is likely to slow down the other uses of LogwrtResult. But I wonder if
a better path forward would be to base further work on my older
uncommitted patch to
On Tue, Jan 30, 2024 at 11:01 PM Alvaro Herrera wrote:
>
> Hmm, this looks quite nice and simple.
Thanks for looking at it.
> My only comment is that a
> sequence like this
>
>/* Read from WAL buffers, if available. */
>rbytes = XLogReadFromBuffers(_message.data[output_message.len],
>
Hmm, this looks quite nice and simple. My only comment is that a
sequence like this
/* Read from WAL buffers, if available. */
rbytes = XLogReadFromBuffers(_message.data[output_message.len],
startptr, nbytes, xlogreader->seg.ws_tli);
output_message.len +=
On Sat, Jan 27, 2024 at 1:04 AM Jeff Davis wrote:
>
> All of these things are functionally equivalent -- the same thing is
> happening at the end. This is just a discussion about API style and how
> that will interact with hypothetical callers that don't exist today.
> And it can also be easily
On Fri, 2024-01-26 at 19:31 +0530, Bharath Rupireddy wrote:
> Are you suggesting to error out instead of returning 0?
We'd do neither of those things, because no caller should actually call
it while RecoveryInProgress() or on a different timeline.
> How about
> returning a negative value
On Fri, Jan 26, 2024 at 8:31 AM Jeff Davis wrote:
>
> > PSA v20 patch set.
>
> 0001 is very close. I have the following suggestions:
>
> * Don't just return zero. If the caller is doing something we don't
> expect, we want to fix the caller. I understand you'd like this to be
> more like a
On Thu, 2024-01-25 at 14:35 +0530, Bharath Rupireddy wrote:
>
> "expecting zeros at the end" - this can't always be true as the WAL
>
...
> I think this needs to be discussed separately. If okay, I'll start a
> new thread.
Thank you for investigating. When the above issue is handled, I'll be
On Tue, Jan 23, 2024 at 9:37 AM Jeff Davis wrote:
>
> On Mon, 2024-01-22 at 12:12 -0800, Andres Freund wrote:
> > I still think that anything that requires such checks shouldn't be
> > merged. It's completely bogus to check page contents for validity
> > when we
> > should have metadata telling
On Mon, 2024-01-22 at 12:12 -0800, Andres Freund wrote:
> I still think that anything that requires such checks shouldn't be
> merged. It's completely bogus to check page contents for validity
> when we
> should have metadata telling us which range of the buffers is valid
> and which
> not.
The
Hi,
On 2024-01-10 19:59:29 +0530, Bharath Rupireddy wrote:
> + /*
> + * Typically, we must not read a WAL buffer page that just got
> + * initialized. Because we waited enough for the in-progress WAL
> + * insertions to finish above. However,
Hi Bharath,
Thanks for working on this. It seems like a nice improvement to have.
Here are some comments on 0001 patch.
1- xlog.c
+ /*
+ * Fast paths for the following reasons: 1) WAL buffers aren't in use when
+ * server is in recovery. 2) WAL is inserted into WAL buffers on current
+ *
On Wed, 2024-01-10 at 19:59 +0530, Bharath Rupireddy wrote:
> I've addressed the above review comments and attached v19 patch-set.
Regarding:
- if (!WALRead(state, cur_page, targetPagePtr, XLOG_BLCKSZ, tli,
-))
+ if (!WALRead(state, cur_page,
On Fri, Jan 5, 2024 at 7:20 AM Jeff Davis wrote:
>
> On Wed, 2023-12-20 at 15:36 +0530, Bharath Rupireddy wrote:
> > Thanks. Attaching remaining patches as v18 patch-set after commits
> > c3a8e2a7cb16 and 766571be1659.
>
> Comments:
Thanks for reviewing.
> I still think the right thing for this
On Wed, 2023-12-20 at 15:36 +0530, Bharath Rupireddy wrote:
> Thanks. Attaching remaining patches as v18 patch-set after commits
> c3a8e2a7cb16 and 766571be1659.
Comments:
I still think the right thing for this patch is to call
XLogReadFromBuffers() directly from the callers who need it, and not
On Fri, Dec 8, 2023 at 6:04 AM Jeff Davis wrote:
>
> On Thu, 2023-12-07 at 15:59 +0530, Bharath Rupireddy wrote:
> > In the attached v17 patch
>
> The code for 0001 itself looks good. These are minor concerns and I am
> inclined to commit something like it fairly soon.
Thanks. Attaching
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
On Mon, Nov 13, 2023 at 7:02 PM Bharath Rupireddy
wrote:
>
> On Fri, Nov 10, 2023 at 2:28 AM Andres Freund wrote:
>
> > I think the code needs to make sure that *never* happens. That seems
> > unrelated
> > to holding or not holding WALBufMappingLock. Even if the page header is
> > already
On Fri, Nov 10, 2023 at 2:28 AM Andres Freund wrote:
>
> On 2023-11-08 13:10:34 +0530, Bharath Rupireddy wrote:
> > > > + /*
> > > > + * The fact that we acquire WALBufMappingLock while
> > > > reading the WAL
> > > > + * buffer page itself guarantees that
Hi,
On 2023-11-08 13:10:34 +0530, Bharath Rupireddy wrote:
> > > + /*
> > > + * The fact that we acquire WALBufMappingLock while reading
> > > the WAL
> > > + * buffer page itself guarantees that no one else
> > > initializes it or
> > > + *
On Sat, Nov 4, 2023 at 6:13 AM Andres Freund wrote:
>
> > + cur_lsn = GetFlushRecPtr(NULL);
> > + if (unlikely(startptr > cur_lsn))
> > + elog(ERROR, "WAL start LSN %X/%X specified for reading from
> > WAL buffers must be less than current database system WAL LSN %X/%X",
> >
On Sun, Nov 5, 2023 at 2:57 AM Jeff Davis wrote:
>
> On Sat, 2023-11-04 at 20:55 +0530, Bharath Rupireddy wrote:
> > + XLogRecPtr EndPtr =
> > pg_atomic_read_u64(>xlblocks[curridx]);
> > +
> > + /*
> > + * xlblocks value can be InvalidXLogRecPtr before
>
On Sat, 2023-11-04 at 20:55 +0530, Bharath Rupireddy wrote:
> + XLogRecPtr EndPtr =
> pg_atomic_read_u64(>xlblocks[curridx]);
> +
> + /*
> + * xlblocks value can be InvalidXLogRecPtr before
> the new WAL buffer
> + * page gets initialized in
On Sat, Nov 4, 2023 at 1:17 AM Jeff Davis wrote:
>
> > > I think it needs something like:
> > >
> > > pg_atomic_write_u64(>xlblocks[nextidx],
> > > InvalidXLogRecPtr);
> > > pg_write_barrier();
> > >
> > > before the MemSet.
> >
> > I think it works. First, xlblocks needs to be turned to an
Hi,
On 2023-11-02 22:38:38 +0530, Bharath Rupireddy wrote:
> From 5b5469d7dcd8e98bfcaf14227e67356bbc1f5fe8 Mon Sep 17 00:00:00 2001
> From: Bharath Rupireddy
> Date: Thu, 2 Nov 2023 15:10:51 +
> Subject: [PATCH v14] Track oldest initialized WAL buffer page
>
> ---
>
Hi,
On 2023-11-03 20:23:30 +0530, Bharath Rupireddy wrote:
> On Fri, Nov 3, 2023 at 12:35 PM Jeff Davis wrote:
> >
> > On Thu, 2023-11-02 at 22:38 +0530, Bharath Rupireddy wrote:
> > > > I suppose the question is: should reading from the WAL buffers an
> > > > intentional thing that the caller
On Fri, 2023-11-03 at 20:23 +0530, Bharath Rupireddy wrote:
> >
> OldestInitializedPage is introduced in v14-0001 patch. Please have a
> look.
I don't see why that's necessary if we move to the algorithm I
suggested below that doesn't require a lock.
> >
> Okay. Current patch doesn't support
On Fri, Nov 3, 2023 at 12:35 PM Jeff Davis wrote:
>
> On Thu, 2023-11-02 at 22:38 +0530, Bharath Rupireddy wrote:
> > > I suppose the question is: should reading from the WAL buffers an
> > > intentional thing that the caller does explicitly by specific
> > > callers?
> > > Or is it an
On Thu, 2023-11-02 at 22:38 +0530, Bharath Rupireddy wrote:
> > I suppose the question is: should reading from the WAL buffers an
> > intentional thing that the caller does explicitly by specific
> > callers?
> > Or is it an optimization that should be hidden from the caller?
> >
> > I tend
On Sat, Oct 28, 2023 at 2:22 AM Jeff Davis wrote:
>
> I think I see what you are saying: WALRead() is at a lower level than
> the XLogReaderRoutine callbacks, because it's used by the .page_read
> callbacks.
>
> That makes sense, but my first interpretation was that WALRead() is
> above the
On Fri, 2023-10-27 at 03:46 +0530, Bharath Rupireddy wrote:
>
> Almost all the available backend
> page_read callbacks read_local_xlog_page_no_wait,
> read_local_xlog_page, logical_read_xlog_page except XLogPageRead
> (which is used for recovery when WAL buffers aren't used at all) have
> one
On Wed, Oct 25, 2023 at 5:45 AM Jeff Davis wrote:
>
> Comments:
Thanks for reviewing.
> * It would be good to document that this is partially an optimization
> (read from memory first) and partially an API difference that allows
> reading unflushed data. For instance, walsender may benefit
>
On Tue, Oct 24, 2023 at 05:15:19PM -0700, Jeff Davis wrote:
> * Are you sure that reducing the number of calls to memcpy() is a win?
> I would expect that to be true only if the memcpy()s are tiny, but here
> they are around XLOG_BLCKSZ. I believe this was done based on a comment
> from Nathan
On Sat, 2023-10-21 at 23:59 +0530, Bharath Rupireddy wrote:
> I'm attaching v12 patch set with just pgperltidy ran on the new TAP
> test added in 0002. No other changes from that of v11 patch set.
Thank you.
Comments:
* It would be good to document that this is partially an optimization
(read
On Fri, Oct 20, 2023 at 10:19 PM Bharath Rupireddy
wrote:
>
> On Thu, Oct 12, 2023 at 4:13 AM Andres Freund wrote:
> >
> > On 2023-10-03 16:05:32 -0700, Jeff Davis wrote:
> > > On Sat, 2023-01-14 at 12:34 -0800, Andres Freund wrote:
> > > > One benefit would be that it'd make it more realistic
On Thu, Oct 12, 2023 at 4:13 AM Andres Freund wrote:
>
> On 2023-10-03 16:05:32 -0700, Jeff Davis wrote:
> > On Sat, 2023-01-14 at 12:34 -0800, Andres Freund wrote:
> > > One benefit would be that it'd make it more realistic to use direct
> > > IO for WAL
> > > - for which I have seen significant
On Thu, Oct 12, 2023 at 4:13 AM Andres Freund wrote:
>
> On 2023-10-03 16:05:32 -0700, Jeff Davis wrote:
> >
> > Does this patch still look like a good fit for your (or someone else's)
> > plans for direct IO here? If so, would committing this soon make it
> > easier to make progress on that, or
Hi,
On 2023-10-03 16:05:32 -0700, Jeff Davis wrote:
> On Sat, 2023-01-14 at 12:34 -0800, Andres Freund wrote:
> > One benefit would be that it'd make it more realistic to use direct
> > IO for WAL
> > - for which I have seen significant performance benefits. But when we
> > afterwards have to
On Sat, 2023-01-14 at 12:34 -0800, Andres Freund wrote:
> One benefit would be that it'd make it more realistic to use direct
> IO for WAL
> - for which I have seen significant performance benefits. But when we
> afterwards have to re-read it from disk to replicate, it's less
> clearly a win.
On Tue, Mar 7, 2023 at 11:14 PM Nathan Bossart wrote:
>
> On Tue, Mar 07, 2023 at 12:39:13PM +0530, Bharath Rupireddy wrote:
> > On Tue, Mar 7, 2023 at 3:30 AM Nathan Bossart
> > wrote:
> >> Is it possible to memcpy more than a page at a time?
> >
> > It would complicate things a lot there; the
On Sun, Mar 12, 2023 at 11:00 PM Bharath Rupireddy
wrote:
>
> I have some review comments to fix on
> v8-0001, so, I'll be sending out v9 patch-set soon.
Please find the attached v9 patch set for further review. I moved the
check for just-initialized WAL buffer pages before reading the page.
Up
On Sun, Mar 12, 2023 at 12:52 AM Nitin Jadhav
wrote:
>
> I went through the v8 patch.
Thanks for looking at it. Please post the responses in-line, not above
the entire previous message for better readability.
> Following are my thoughts to improve the
> WAL buffer hit ratio.
Note that the
> [1]
> subscription tests:
> PATCHED: WAL buffers hit - 1972, misses - 32616
Can you share more details about the test here?
I went through the v8 patch. Following are my thoughts to improve the
WAL buffer hit ratio.
Currently the no-longer-needed WAL data present in WAL buffers gets
cleared
On Tue, Mar 07, 2023 at 12:39:13PM +0530, Bharath Rupireddy wrote:
> On Tue, Mar 7, 2023 at 3:30 AM Nathan Bossart
> wrote:
>> Is it possible to memcpy more than a page at a time?
>
> It would complicate things a lot there; the logic to figure out the
> last page bytes that may or may not fit
On Tue, Mar 7, 2023 at 3:30 AM Nathan Bossart wrote:
>
> +void
> +XLogReadFromBuffers(XLogRecPtr startptr,
>
> Since this function presently doesn't return anything, can we have it
> return the number of bytes read instead of storing it in a pointer
> variable?
Done.
> +ptr = startptr;
> +
+void
+XLogReadFromBuffers(XLogRecPtr startptr,
+TimeLineID tli,
+Size count,
+char *buf,
+Size *read_bytes)
Since this function presently doesn't return anything, can we have it
return the number of bytes read
On Wed, Mar 1, 2023 at 9:45 AM Nathan Bossart wrote:
>
> On Tue, Feb 28, 2023 at 10:38:31AM +0530, Bharath Rupireddy wrote:
> > On Tue, Feb 28, 2023 at 6:14 AM Nathan Bossart
> > wrote:
> >> Why do we only read a page at a time in XLogReadFromBuffersGuts()? What is
> >> preventing us from
On Wed, Mar 1, 2023 at 2:39 PM Bharath Rupireddy
wrote:
>
> Please find the attached v5 patch set for further review.
I simplified the code largely by moving the logic of reading the WAL
buffer page from a separate function to the main while loop. This
enabled me to get rid of
On Wed, Mar 1, 2023 at 12:06 AM Kuntal Ghosh wrote:
>
> On Tue, Feb 28, 2023 at 10:38 AM Bharath Rupireddy
> wrote:
> >
> +/*
> + * Guts of XLogReadFromBuffers().
> + *
> + * Read 'count' bytes into 'buf', starting at location 'ptr', from WAL
> + * fetched WAL buffers on timeline 'tli' and
On Tue, Feb 28, 2023 at 10:38:31AM +0530, Bharath Rupireddy wrote:
> On Tue, Feb 28, 2023 at 6:14 AM Nathan Bossart
> wrote:
>> Why do we only read a page at a time in XLogReadFromBuffersGuts()? What is
>> preventing us from copying all the data we need in one go?
>
> Note that most of the
On Tue, Feb 28, 2023 at 10:38 AM Bharath Rupireddy
wrote:
>
+/*
+ * Guts of XLogReadFromBuffers().
+ *
+ * Read 'count' bytes into 'buf', starting at location 'ptr', from WAL
+ * fetched WAL buffers on timeline 'tli' and return the read bytes.
+ */
s/fetched WAL buffers/fetched from WAL buffers
On Tue, Feb 28, 2023 at 6:14 AM Nathan Bossart wrote:
>
> On Wed, Feb 08, 2023 at 08:00:00PM +0530, Bharath Rupireddy wrote:
> > + /*
> > + * We read some of the requested bytes. Continue to
> > read remaining
> > + * bytes.
> > +
On Wed, Feb 08, 2023 at 08:00:00PM +0530, Bharath Rupireddy wrote:
> + /*
> + * We read some of the requested bytes. Continue to
> read remaining
> + * bytes.
> + */
> + ptr += nread;
> +
On Wed, Feb 8, 2023 at 10:33 AM Dilip Kumar wrote:
>
> On Wed, Feb 8, 2023 at 9:57 AM Bharath Rupireddy
> wrote:
> >
> > I can also do a few other things, but before working on them, I'd like
> > to hear from others:
> > 1. A separate wait event (WAIT_EVENT_WAL_READ_FROM_BUFFERS) for
> > reading
On Wed, Feb 8, 2023 at 9:57 AM Bharath Rupireddy
wrote:
>
> I can also do a few other things, but before working on them, I'd like
> to hear from others:
> 1. A separate wait event (WAIT_EVENT_WAL_READ_FROM_BUFFERS) for
> reading from WAL buffers - right now, WAIT_EVENT_WAL_READ is being
> used
On Tue, Feb 7, 2023 at 4:12 PM Dilip Kumar wrote:
>
> On Mon, Dec 26, 2022 at 2:20 PM Bharath Rupireddy
> wrote:
>
> I have gone through this patch, I have some comments (mostly cosmetic
> and comments)
Thanks a lot for reviewing.
> From the above comments, it appears that we are reading from
On Mon, Dec 26, 2022 at 2:20 PM Bharath Rupireddy
wrote:
>
I have gone through this patch, I have some comments (mostly cosmetic
and comments)
1.
+ /*
+ * We have found the WAL buffer page holding the given LSN. Read from a
+ * pointer to the right offset within the page.
+ */
+ memcpy(page,
On Fri, Jan 27, 2023 at 12:16 PM Masahiko Sawada wrote:
>
> I'd like to confirm whether there is any performance regression caused
> by this patch in some cases, especially when not using DIO.
Thanks. I ran some insert tests with primary and 1 async standby.
Please see the numbers below and
On Fri, Jan 27, 2023 at 3:17 PM Andres Freund wrote:
>
> Hi,
>
> On 2023-01-27 14:24:51 +0900, Masahiko Sawada wrote:
> > If I'm understanding this result correctly, it seems to me that your
> > patch works well with the WAL DIO patch (WALDIO vs. WAL DIO & WAL
> > BUFFERS READ), but there seems
Hi,
On 2023-01-27 14:24:51 +0900, Masahiko Sawada wrote:
> If I'm understanding this result correctly, it seems to me that your
> patch works well with the WAL DIO patch (WALDIO vs. WAL DIO & WAL
> BUFFERS READ), but there seems no visible performance gain with only
> your patch (HEAD vs. WAL
On Thu, Jan 26, 2023 at 2:33 PM Bharath Rupireddy
wrote:
>
> On Thu, Jan 26, 2023 at 2:45 AM Andres Freund wrote:
> >
> > Hi,
> >
> > On 2023-01-14 12:34:03 -0800, Andres Freund wrote:
> > > On 2023-01-14 00:48:52 -0800, Jeff Davis wrote:
> > > > On Mon, 2022-12-26 at 14:20 +0530, Bharath
On Thu, Jan 26, 2023 at 2:45 AM Andres Freund wrote:
>
> Hi,
>
> On 2023-01-14 12:34:03 -0800, Andres Freund wrote:
> > On 2023-01-14 00:48:52 -0800, Jeff Davis wrote:
> > > On Mon, 2022-12-26 at 14:20 +0530, Bharath Rupireddy wrote:
> > > > Please review the attached v2 patch further.
> > >
> >
On Sat, Jan 14, 2023 at 12:34 PM Andres Freund wrote:
> Hi,
>
> On 2023-01-14 00:48:52 -0800, Jeff Davis wrote:
> > On Mon, 2022-12-26 at 14:20 +0530, Bharath Rupireddy wrote:
> > > Please review the attached v2 patch further.
> >
> > I'm still unclear on the performance goals of this patch. I
Hi,
On 2023-01-14 12:34:03 -0800, Andres Freund wrote:
> On 2023-01-14 00:48:52 -0800, Jeff Davis wrote:
> > On Mon, 2022-12-26 at 14:20 +0530, Bharath Rupireddy wrote:
> > > Please review the attached v2 patch further.
> >
> > I'm still unclear on the performance goals of this patch. I see that
Hi,
On 2023-01-14 00:48:52 -0800, Jeff Davis wrote:
> On Mon, 2022-12-26 at 14:20 +0530, Bharath Rupireddy wrote:
> > Please review the attached v2 patch further.
>
> I'm still unclear on the performance goals of this patch. I see that it
> will reduce syscalls, which sounds good, but to what
On Mon, 2022-12-26 at 14:20 +0530, Bharath Rupireddy wrote:
> Please review the attached v2 patch further.
I'm still unclear on the performance goals of this patch. I see that it
will reduce syscalls, which sounds good, but to what end?
Does it allow a greater number of walsenders? Lower
On Sun, Dec 25, 2022 at 4:55 PM Dilip Kumar wrote:
>
> > > This adds copying of the whole page (at least) at every WAL *record*
> > > read,
> >
> > In the worst case yes, but that may not always be true. On a typical
> > production server with decent write traffic, it happens that the
> > callers
On Fri, Dec 23, 2022 at 3:46 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 12, 2022 at 8:27 AM Kyotaro Horiguchi
> wrote:
> >
>
> Thanks for providing thoughts.
>
> > At Fri, 9 Dec 2022 14:33:39 +0530, Bharath Rupireddy
> > wrote in
> > > The patch introduces concurrent readers for the WAL
On Mon, Dec 12, 2022 at 8:27 AM Kyotaro Horiguchi
wrote:
>
Thanks for providing thoughts.
> At Fri, 9 Dec 2022 14:33:39 +0530, Bharath Rupireddy
> wrote in
> > The patch introduces concurrent readers for the WAL buffers, so far
> > only there are concurrent writers. In the patch, WALRead()
Sorry for the confusion.
At Mon, 12 Dec 2022 12:06:36 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Mon, 12 Dec 2022 11:57:17 +0900 (JST), Kyotaro Horiguchi
> wrote in
> > This patch copies the bleeding edge WAL page without recording the
> > (next) insertion point nor checking whether all
At Mon, 12 Dec 2022 11:57:17 +0900 (JST), Kyotaro Horiguchi
wrote in
> This patch copies the bleeding edge WAL page without recording the
> (next) insertion point nor checking whether all in-progress insertion
> behind the target LSN have finished. Thus the copied page may have
> holes. That
At Fri, 9 Dec 2022 14:33:39 +0530, Bharath Rupireddy
wrote in
> The patch introduces concurrent readers for the WAL buffers, so far
> only there are concurrent writers. In the patch, WALRead() takes just
> one lock (WALBufMappingLock) in shared mode to enable concurrent
> readers and does
Hi,
WALRead() currently reads WAL from the WAL file on the disk, which
means, the walsenders serving streaming and logical replication
(callers of WALRead()) will have to hit the disk/OS's page cache for
reading the WAL. This may increase the amount of read IO required for
all the walsenders put
95 matches
Mail list logo
