On Wed, Sep 24, 2025 at 7:58 AM Dilip Kumar wrote:
>
> On Fri, Sep 19, 2025 at 10:58 AM Michael Paquier wrote:
> >
> > Do we want to make the order of the checks to be more consistent in
> > both routines? These would require a separate set of double-checks
> > and review, but while we're lookin
On Thu, Sep 25, 2025 at 3:53 AM Michael Paquier wrote:
>
> On Wed, Sep 24, 2025 at 05:33:08PM -0400, Melanie Plageman wrote:
>
> > And, if I'm not mistaken, there was another idea mentioned in [1]
> > which was to move to using GetRecoveryState() in XLogNeedsFlush() and
> > UpdateMinRecoveryPoint
On Mon, Sep 22, 2025 at 08:09:18AM -0400, Melanie Plageman wrote:
> On Fri, Sep 19, 2025 at 1:28 AM Michael Paquier wrote:
>> Do we want to make the order of the checks to be more consistent in
>> both routines? These would require a separate set of double-checks
>> and review, but while we're lo
On Thu, Sep 25, 2025 at 04:53:29PM +0900, Michael Paquier wrote:
> I'd rather tackle each thing separately.
I have just applied the patch to reorder the checks as of
bb68cde4136b. If you have patches for the rest, feel free to submit
them.
--
Michael
signature.asc
Description: PGP signature
On Fri, Sep 19, 2025 at 1:28 AM Michael Paquier wrote:
>
> Do we want to make the order of the checks to be more consistent in
> both routines? These would require a separate set of double-checks
> and review, but while we're looking at this area of the code we may as
> tweak it more..
That make
On Wed, Sep 24, 2025 at 05:33:08PM -0400, Melanie Plageman wrote:
> I think he's referring to the order of checks inside
> UpdateMinRecoveryPoint(). Something like the attached patch.
Yep, thanks. What you are doing with XLogNeedsFlush() looks correct.
> And, if I'm not mistaken, there was anoth
On Fri, Sep 19, 2025 at 10:58 AM Michael Paquier wrote:
>
> On Thu, Sep 18, 2025 at 05:07:00PM +0530, Dilip Kumar wrote:
> > I think this comment is a side note which is stating that it is
> > possible that while XLogNeedFlush() is deciding that based on the
> > current flush position or min recov
On Wed, Sep 10, 2025 at 3:58 AM Michael Paquier wrote:
>
> On Wed, Sep 10, 2025 at 12:48:21PM +0530, Dilip Kumar wrote:
> >> So, looking at the code, it seems like most places we examine
> >> ControlFile->minRecoveryPoint, we condition it on "InArchiveRecovery".
> >> Is this something we want to d
On Tue, Sep 9, 2025 at 9:50 PM Jeff Davis wrote:
>
> On Tue, 2025-09-09 at 16:29 -0400, Melanie Plageman wrote:
>
> > Though, it seems like LocalMinRecoveryPoint must be getting
> > incorrectly set elsewhere, otherwise this would have guarded us from
> > examining the control file:
>
> I am confus
On Mon, Sep 15, 2025 at 8:28 PM Melanie Plageman
wrote:
>
> On Sun, Sep 14, 2025 at 4:10 AM Dilip Kumar wrote:
> >
> > OTOH, when creating a RestartPoint during standby replay or doing
> > archive recovery, access to both LocalMinRecoveryPoint and
> > ControlFile->minRecoveryPoint is correct. By
On Fri, Sep 12, 2025 at 10:02 PM Jeff Davis wrote:
>
> On Fri, 2025-09-12 at 14:04 +0900, Michael Paquier wrote:
> > On Fri, Sep 12, 2025 at 10:21:27AM +0530, Dilip Kumar wrote:
> > > Yeah, asserting it at the end makes sense, as we can ensure that
> > > XLogFlush() and XLogNeedsFlush() agree on t
On Wed, Sep 10, 2025 at 3:18 AM Dilip Kumar wrote:
>
> On Wed, Sep 10, 2025 at 1:59 AM Melanie Plageman
> wrote:
>
> > Though, it seems like LocalMinRecoveryPoint must be getting
> > incorrectly set elsewhere, otherwise this would have guarded us from
> > examining the control file:
> >
> >
On Thu, Sep 18, 2025 at 05:07:00PM +0530, Dilip Kumar wrote:
> I think this comment is a side note which is stating that it is
> possible that while XLogNeedFlush() is deciding that based on the
> current flush position or min recovery point parallely someone might
> flush beyond that point. And i
On Thu, Sep 18, 2025 at 9:24 AM Michael Paquier wrote:
>
> On Wed, Sep 17, 2025 at 09:23:05PM -0400, Melanie Plageman wrote:
> > In terms of comments, I think it is best to update the comment above
> > XLogNeedsFlush(). Something like :
> >
> > /*
> > - * Test whether XLOG data has been flushed u
On Wed, Sep 17, 2025 at 09:23:05PM -0400, Melanie Plageman wrote:
> In terms of comments, I think it is best to update the comment above
> XLogNeedsFlush(). Something like :
>
> /*
> - * Test whether XLOG data has been flushed up to (at least) the given
> position.
> + * Test whether XLOG data h
On Wed, Sep 17, 2025 at 7:20 PM Michael Paquier wrote:
>
> On Tue, Sep 16, 2025 at 09:40:50AM +0900, Michael Paquier wrote:
> > As a whole, the patch looks like a good balance, able to satisfy the
> > new case you want to handle, Melanie. I am guessing that you'd want
> > to tweak it and apply it
> On Sep 18, 2025, at 07:20, Michael Paquier wrote:
>
> On Tue, Sep 16, 2025 at 09:40:50AM +0900, Michael Paquier wrote:
>> As a whole, the patch looks like a good balance, able to satisfy the
>> new case you want to handle, Melanie. I am guessing that you'd want
>> to tweak it and apply it yo
On Tue, Sep 16, 2025 at 09:40:50AM +0900, Michael Paquier wrote:
> As a whole, the patch looks like a good balance, able to satisfy the
> new case you want to handle, Melanie. I am guessing that you'd want
> to tweak it and apply it yourself, so please feel free.
Hearing nothing, I'd like to move
On Wed, Sep 17, 2025 at 07:05:32PM +0530, Dilip Kumar wrote:
> In a recovery scenario, the ControlFile->minRecoveryPoint on a standby
> server is continuously updated. This ensures that even in the event of
> a crash, a valid recovery point is available. However, if a crashed
> standalone server is
On Mon, Sep 15, 2025 at 03:02:36PM -0400, Melanie Plageman wrote:
> As such, we should clarify the comment above the assert in your patch
> to make it clear that the point of the assert is not to check the
> logic in XLogFlush() but to protect against drift between
> XLogNeedsFlush() and XLogFlush(
On Mon, Sep 15, 2025 at 10:58 AM Melanie Plageman
wrote:
>
> On Sun, Sep 14, 2025 at 4:10 AM Dilip Kumar wrote:
> >
> > Updated as per suggestion
>
> I like the idea of adding an assert to keep the two in sync. However, two
> things
>
> 1) Since XLogNeedsFlush() refreshes LogwrtResult, is it pos
On Sun, Sep 14, 2025 at 4:10 AM Dilip Kumar wrote:
>
> On Fri, Sep 12, 2025 at 10:02 PM Jeff Davis wrote:
>
> > Assuming I'm right so far, then I agree with changing the test in
> > XLogNeedsFlush() to !XLogInsertAllowed(), as the patch does.
> >
> > The comment in the patch is describing what's
On Sun, Sep 14, 2025 at 11:23 PM Jeff Davis wrote:
>
> On Sun, 2025-09-14 at 13:39 +0530, Dilip Kumar wrote:
> > I tried to improve it in v2
>
> Thank you. Looks good to me.
Thanks
> >
> > IMHO during crash recovery LocalMinRecoveryPoint and
> > ControlFile->minRecoveryPoint can not initialized
On Sun, 2025-09-14 at 13:39 +0530, Dilip Kumar wrote:
> I tried to improve it in v2
Thank you. Looks good to me.
>
> IMHO during crash recovery LocalMinRecoveryPoint and
> ControlFile->minRecoveryPoint can not initialized until we replay all
> WAL so those should never get accessed. However if
On Fri, 2025-09-12 at 14:04 +0900, Michael Paquier wrote:
> On Fri, Sep 12, 2025 at 10:21:27AM +0530, Dilip Kumar wrote:
> > Yeah, asserting it at the end makes sense, as we can ensure that
> > XLogFlush() and XLogNeedsFlush() agree on the same thing without
> > adding additional overhead. Here is
On Fri, Sep 12, 2025 at 10:21:27AM +0530, Dilip Kumar wrote:
> Yeah, asserting it at the end makes sense, as we can ensure that
> XLogFlush() and XLogNeedsFlush() agree on the same thing without
> adding additional overhead. Here is the revised one.
Melanie and Jeff, what do you think?
--
Michael
On Fri, Sep 12, 2025 at 9:47 AM Michael Paquier wrote:
>
> On Fri, Sep 12, 2025 at 08:45:36AM +0530, Dilip Kumar wrote:
> > On Fri, Sep 12, 2025 at 8:07 AM Dilip Kumar wrote:
> >> +1, it really makes XLogFlush() to directly check using
> >> XLogNeedsFlush() after adding the "WAL inserts are allow
On Fri, Sep 12, 2025 at 08:45:36AM +0530, Dilip Kumar wrote:
> On Fri, Sep 12, 2025 at 8:07 AM Dilip Kumar wrote:
>> +1, it really makes XLogFlush() to directly check using
>> XLogNeedsFlush() after adding the "WAL inserts are allowed" check in
>> XLogNeedsFlush(), this is the best way to avoid an
On Fri, Sep 12, 2025 at 8:07 AM Dilip Kumar wrote:
>
> On Thu, Sep 11, 2025 at 4:51 AM Michael Paquier wrote:
> >
> > On Wed, Sep 10, 2025 at 09:58:08AM -0700, Jeff Davis wrote:
> > > It seems like XLogFlush() and XLogNeedsFlush() should use the same
> > > test, otherwise you could always get som
On Thu, Sep 11, 2025 at 4:51 AM Michael Paquier wrote:
>
> On Wed, Sep 10, 2025 at 09:58:08AM -0700, Jeff Davis wrote:
> > It seems like XLogFlush() and XLogNeedsFlush() should use the same
> > test, otherwise you could always get some confusing inconsistency.
> > Right?
>
> Even if the checks are
On Wed, Sep 10, 2025 at 09:58:08AM -0700, Jeff Davis wrote:
> It seems like XLogFlush() and XLogNeedsFlush() should use the same
> test, otherwise you could always get some confusing inconsistency.
> Right?
Even if the checks are duplicated (dependency could be documented as
well), it would make s
On Wed, Sep 10, 2025 at 11:12:37AM -0400, Melanie Plageman wrote:
> On Wed, Sep 10, 2025 at 3:58 AM Michael Paquier wrote:
>> You have a good point here, especially knowing that for
>> CHECKPOINT_END_OF_RECOVERY case in CreateCheckPoint(), called by the
>> checkpointer, we allow WAL inserts with L
On Wed, 2025-09-10 at 11:12 -0400, Melanie Plageman wrote:
> So, would you consider the defining characteristic of whether or not
> we should use the flush pointer instead of min recovery point in
> XLogNeedsFlush() to be whether or not WAL inserts are allowed?
That was my question here:
https://
On Wed, Sep 10, 2025 at 12:48:21PM +0530, Dilip Kumar wrote:
>> So, looking at the code, it seems like most places we examine
>> ControlFile->minRecoveryPoint, we condition it on "InArchiveRecovery".
>> Is this something we want to do in XLogNeedsFlush() to avoid reading
>> from ControlFile->minRec
On Wed, Sep 10, 2025 at 1:59 AM Melanie Plageman
wrote:
>
> Thanks for taking time to reply!
>
> On Wed, Sep 3, 2025 at 11:38 PM Dilip Kumar wrote:
> >
> > On Sat, Aug 30, 2025 at 4:16 AM Melanie Plageman
> > wrote:
> >
> > > Why is it okay for other processes than the startup process to
> > > i
On Tue, 2025-09-09 at 16:29 -0400, Melanie Plageman wrote:
> On Wed, Sep 3, 2025 at 11:38 PM Dilip Kumar
> wrote:
> >
> > On Sat, Aug 30, 2025 at 4:16 AM Melanie Plageman
> > wrote:
> >
> > > Why is it okay for other processes than the startup process to
> > > initialize LocalMinRecoveryPoint f
Thanks for taking time to reply!
On Wed, Sep 3, 2025 at 11:38 PM Dilip Kumar wrote:
>
> On Sat, Aug 30, 2025 at 4:16 AM Melanie Plageman
> wrote:
>
> > Why is it okay for other processes than the startup process to
> > initialize LocalMinRecoveryPoint from ControlFile->minRecoveryPoint
> > durin
On Sat, Aug 30, 2025 at 4:16 AM Melanie Plageman
wrote:
>
> Hi,
>
> If you call XLogNeedsFlush() immediately after calling XLogFlush() in
> FlushBuffer(), it can return true.
>
> With this diff:
>
> diff --git a/src/backend/storage/buffer/bufmgr.c
> b/src/backend/storage/buffer/bufmgr.c
> index 35
Hi,
If you call XLogNeedsFlush() immediately after calling XLogFlush() in
FlushBuffer(), it can return true.
With this diff:
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
index 350cc0402aa..91c3fe99d6e 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++
39 matches
Mail list logo
