On Thu, Oct 19, 2023 at 1:53 AM Michael Paquier wrote:
> Seems fine to me. Thanks for considering the idea.
I think it was a good idea!
I've committed the patch.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Oct 18, 2023 at 10:24:50AM -0400, Robert Haas wrote:
> I added a variant of this test case. Here's v10.
+-- Verify that an XLOG_CHECKPOINT_REDO record begins at precisely the redo LSN
+-- of the checkpoint we just performed.
+SELECT redo_lsn FROM pg_control_checkpoint() \gset
+SELECT start
On Tue, Oct 17, 2023 at 8:35 PM Michael Paquier wrote:
> Suggestion is from here, with a test for pg_walinspect after it runs
> its online checkpoint (see the full-page case):
> https://www.postgresql.org/message-id/ZOvf1tu6rfL/b...@paquier.xyz
>
> +-- Check presence of REDO record.
> +SELECT redo
On Tue, Oct 17, 2023 at 12:45:52PM -0400, Robert Haas wrote:
> On Fri, Oct 13, 2023 at 3:29 AM Michael Paquier wrote:
>> I've mentioned as well a test in pg_walinspect after one of the
>> checkpoints generated there, but what you do here is enough for the
>> online case.
>
> I don't quite underst
On Fri, Oct 13, 2023 at 3:29 AM Michael Paquier wrote:
> Now looking at 0002, where you should be careful about the code
> indentation or koel will complain.
Fixed in the attached version.
> This makes the new code call LocalSetXLogInsertAllowed() and what we
> set for checkPoint.PrevTimeLineID
On Tue, Oct 10, 2023 at 02:43:34PM -0400, Robert Haas wrote:
> Here's a new patch set. I think I've incorporated the performance
> fixes that you've suggested so far into this version. I also adjusted
> a couple of other things:
Now looking at 0002, where you should be careful about the code
inden
On Thu, Oct 12, 2023 at 3:27 AM Michael Paquier wrote:
> I have looked at 0001, for now.. And it looks OK to me.
Cool. I've committed that one. Thanks for the review.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Oct 10, 2023 at 02:43:34PM -0400, Robert Haas wrote:
> - I combined what were previously 0002 and 0003 into a single patch,
> since that's how this would get committed.
>
> - I fixed up some comments.
>
> - I updated commit messages.
>
> Hopefully this is getting close to good enough.
I
On Tue, Oct 10, 2023 at 11:33 AM Robert Haas wrote:
> On Mon, Oct 9, 2023 at 4:47 PM Andres Freund wrote:
> > I think we might be able to speed some of this up by pre-compute values so
> > we
> > can implement things like bytesleft / UsableBytesInPage with shifts. IIRC we
> > already insist on p
On Mon, Oct 9, 2023 at 4:47 PM Andres Freund wrote:
> As noted in my email from a few minutes ago, I agree that optimizing this
> shouldn't be a requirement for merging the patch.
Here's a new patch set. I think I've incorporated the performance
fixes that you've suggested so far into this versio
Hi,
On 2023-10-09 18:31:11 -0400, Robert Haas wrote:
> On Mon, Oct 9, 2023 at 4:47 PM Andres Freund wrote:
> > I think we might be able to speed some of this up by pre-compute values so
> > we
> > can implement things like bytesleft / UsableBytesInPage with shifts. IIRC we
> > already insist on
On Mon, Oct 9, 2023 at 4:47 PM Andres Freund wrote:
> I think we might be able to speed some of this up by pre-compute values so we
> can implement things like bytesleft / UsableBytesInPage with shifts. IIRC we
> already insist on power-of-two segment sizes, so instead of needing to divide
> by a
Hi,
As noted in my email from a few minutes ago, I agree that optimizing this
shouldn't be a requirement for merging the patch.
On 2023-10-09 15:58:36 -0400, Robert Haas wrote:
> 1. The reason why we're doing this multiplication and division is to
> make sure that the code in ReserveXLogInsertLo
Hi,
On 2023-10-06 13:44:55 -0400, Robert Haas wrote:
> On Thu, Oct 5, 2023 at 2:34 PM Andres Freund wrote:
> > If I add an unlikely around if (rechdr->xl_rmid == RM_XLOG_ID), the
> > performance does improve. But that "only" brings it up to 322.406. Not sure
> > what the rest is.
>
> I don't rea
On Thu, Oct 5, 2023 at 2:34 PM Andres Freund wrote:
> One thing that's notable, but not related to the patch, is that we waste a
> fair bit of cpu time below XLogInsertRecord() with divisions. I think they're
> all due to the use of UsableBytesInSegment in
> XLogBytePosToRecPtr/XLogBytePosToEndRec
On Thu, Oct 5, 2023 at 2:34 PM Andres Freund wrote:
> If I add an unlikely around if (rechdr->xl_rmid == RM_XLOG_ID), the
> performance does improve. But that "only" brings it up to 322.406. Not sure
> what the rest is.
I don't really think this is worth worrying about. A sub-one-percent
regressi
Hi,
On 2023-10-02 10:42:37 -0400, Robert Haas wrote:
> I was trying to think of a test case where XLogInsertRecord would be
> exercised as heavily as possible, so I really wanted to generate a lot
> of WAL while doing as little real work as possible. The best idea that
> I had was to run pg_create
On Wed, Sep 20, 2023 at 4:20 PM Robert Haas wrote:
> Here are some patches.
Here are some updated patches. Following some off-list conversation
with Andres, I restructured 0003 to put the common case first and use
likely(), and I fixed the brown-paper-bag noted by Amit. I then turned
my attention
On Thu, Sep 21, 2023 at 9:06 PM Robert Haas wrote:
>
> On Thu, Sep 21, 2023 at 4:22 AM Amit Kapila wrote:
> > After the 0003 patch, do we need acquire exclusive lock via
> > WALInsertLockAcquireExclusive() for non-shutdown checkpoints. Even the
> > comment "We must block concurrent insertions whi
On Thu, Sep 21, 2023 at 1:50 AM Robert Haas wrote:
>
> On Mon, Sep 18, 2023 at 2:57 PM Robert Haas wrote:
> > I've been brainstorming about this today, trying to figure out some
> > ideas to make it work.
>
> Here are some patches.
>
> 0001 refactors XLogInsertRecord to unify a couple of separate
On Thu, Sep 21, 2023 at 4:22 AM Amit Kapila wrote:
> After the 0003 patch, do we need acquire exclusive lock via
> WALInsertLockAcquireExclusive() for non-shutdown checkpoints. Even the
> comment "We must block concurrent insertions while examining insert
> state to determine the checkpoint REDO p
On Thu, Sep 21, 2023 at 7:05 AM Robert Haas wrote:
>
> On Mon, Sep 18, 2023 at 2:57 PM Robert Haas wrote:
> > I've been brainstorming about this today, trying to figure out some
> > ideas to make it work.
>
> Here are some patches.
>
> 0001 refactors XLogInsertRecord to unify a couple of separate
On Mon, Sep 18, 2023 at 2:57 PM Robert Haas wrote:
> I've been brainstorming about this today, trying to figure out some
> ideas to make it work.
Here are some patches.
0001 refactors XLogInsertRecord to unify a couple of separate tests of
isLogSwitch, hopefully making it cleaner and cheaper to
On Fri, Jul 14, 2023 at 11:16 AM Andres Freund wrote:
> I suspect we might be able to get rid of the need for exclusive inserts
> here. If we rid of that, we could determine the redoa location based on the
> LSN determined by the XLogInsert().
I've been brainstorming about this today, trying to f
On Thu, Aug 31, 2023 at 09:55:45AM +0530, Dilip Kumar wrote:
> Yeah, good catch. With this, it seems like we can not move this new
> WAL Insert out of the Exclusive WAL insertion lock right? Because if
> we want to set the LSN of this record as the checkpoint.redo then
> there should not be any c
On Thu, Aug 31, 2023 at 9:36 AM Michael Paquier wrote:
>
> On Wed, Aug 30, 2023 at 04:51:19PM +0530, Dilip Kumar wrote:
> > Your suggestions LGTM so modified accordingly
>
> I have been putting my HEAD on this patch for a few hours, reviewing
> the surroundings, and somewhat missed that this compu
On Wed, Aug 30, 2023 at 04:51:19PM +0530, Dilip Kumar wrote:
> Your suggestions LGTM so modified accordingly
I have been putting my HEAD on this patch for a few hours, reviewing
the surroundings, and somewhat missed that this computation is done
while we do not hold the WAL insert locks:
+ c
On Wed, Aug 30, 2023 at 1:03 PM Michael Paquier wrote:
>
> On Mon, Aug 28, 2023 at 01:47:18PM +0530, Dilip Kumar wrote:
> > I removed this mainly because now in other comments[1] where we are
> > introducing this new CHECKPOINT_REDO record we are explaining the
> > problem
> > that the redo locati
On Mon, Aug 28, 2023 at 01:47:18PM +0530, Dilip Kumar wrote:
> I removed this mainly because now in other comments[1] where we are
> introducing this new CHECKPOINT_REDO record we are explaining the
> problem
> that the redo location and the actual checkpoint records are not at
> the same place and
On Mon, Aug 28, 2023 at 5:14 AM Michael Paquier wrote:
>
> On Fri, Aug 25, 2023 at 11:08:25AM +0530, Dilip Kumar wrote:
> > Here is the updated version of the patch.
>
> The concept of the patch looks sound to me. I have a few comments.
Thanks for the review
> +* This special record, ho
On Fri, Aug 25, 2023 at 11:08:25AM +0530, Dilip Kumar wrote:
> Here is the updated version of the patch.
The concept of the patch looks sound to me. I have a few comments.
+* This special record, however, is not required when we doing a
shutdown
+* checkpoint, as there will be
On Fri, Aug 18, 2023 at 10:12 AM Dilip Kumar wrote:
>
> On Fri, Aug 18, 2023 at 5:24 AM Michael Paquier wrote:
> >
> > On Thu, Aug 17, 2023 at 01:11:50PM +0530, Dilip Kumar wrote:
> > > Yeah right, actually I was confused, I assumed it will return the
> > > start LSN of the record. And I do not
On Fri, Aug 18, 2023 at 5:24 AM Michael Paquier wrote:
>
> On Thu, Aug 17, 2023 at 01:11:50PM +0530, Dilip Kumar wrote:
> > Yeah right, actually I was confused, I assumed it will return the
> > start LSN of the record. And I do not see any easy way to identify
> > the Start LSN of this record so
On Thu, Aug 17, 2023 at 01:11:50PM +0530, Dilip Kumar wrote:
> Yeah right, actually I was confused, I assumed it will return the
> start LSN of the record. And I do not see any easy way to identify
> the Start LSN of this record so maybe this solution will not work. I
> will have to think of some
On Thu, Aug 17, 2023 at 10:52 AM Michael Paquier wrote:
>
> On Tue, Aug 15, 2023 at 02:23:43PM +0530, Dilip Kumar wrote:
> > Yeah, good idea, actually we can do this insert outside of the
> > exclusive insert lock and set the LSN of this insert as the
> > checkpoint. redo location. So now we do n
On Tue, Aug 15, 2023 at 02:23:43PM +0530, Dilip Kumar wrote:
> Yeah, good idea, actually we can do this insert outside of the
> exclusive insert lock and set the LSN of this insert as the
> checkpoint. redo location. So now we do not need to compute the
> checkpoint. redo based on the current inse
On Fri, Jul 14, 2023 at 8:46 PM Andres Freund wrote:
>
> Hi,
>
> As I think I mentioned before, I like this idea. However, I don't like the
> implementation too much.
Thanks for looking into it.
> This might work right now, but doesn't really seem maintainable. Nor do I like
> adding branches i
Hi,
As I think I mentioned before, I like this idea. However, I don't like the
implementation too much.
On 2023-06-15 13:11:57 +0530, Dilip Kumar wrote:
> diff --git a/src/backend/access/transam/xlog.c
> b/src/backend/access/transam/xlog.c
> index b2430f617c..a025fb91e2 100644
> --- a/src/backen
As discussed [1 ][2] currently, the checkpoint-redo LSN can not be
accurately detected while processing the WAL. Although we have a
checkpoint WAL record containing the exact redo LSN, other WAL records
may be inserted between the checkpoint-redo LSN and the actual
checkpoint record. If we want to
39 matches
Mail list logo