On Mon, Nov 29, 2021 at 10:12 AM Michael Paquier wrote:
>
> On Mon, Nov 29, 2021 at 09:28:23AM +0530, Bharath Rupireddy wrote:
> > In that case, why can't we inline UpdateControlFile to avoid the
> > function call cost? Do you see any issues with it?
>
> This routine is IMO not something worth bot
On Mon, Nov 29, 2021 at 09:28:23AM +0530, Bharath Rupireddy wrote:
> In that case, why can't we inline UpdateControlFile to avoid the
> function call cost? Do you see any issues with it?
This routine is IMO not something worth bothering about.
> BTW, the v6 patch proposed by Amul at [1], looks go
On Sun, Nov 28, 2021 at 10:03 AM Michael Paquier wrote:
> We want to update the
> control file's timestamp when it is written, before calculating its
> CRC.
Okay.
> > Why do we even need UpdateControlFile which just calls another
> > function? It may be there for usability and readability, but c
On Sun, Nov 28, 2021 at 07:53:13AM +0530, Bharath Rupireddy wrote:
> Isn't it better if we update the ControlFile->time at the end of the
> update_controlfile, after file write/sync?
I don't quite understand your point here. We want to update the
control file's timestamp when it is written, befor
On Fri, Nov 26, 2021 at 2:48 PM Amul Sul wrote:
> > ControlFile.state = DB_SHUTDOWNED;
> > - ControlFile.time = (pg_time_t) time(NULL);
> > This one had better not be removed, either, as we require pg_resetwal
> > to guess a set of control file values. Removing the one in
> > RewriteControl
On Fri, Nov 26, 2021 at 12:16 PM Michael Paquier wrote:
>
> On Thu, Nov 25, 2021 at 04:04:23PM +0900, Michael Paquier wrote:
> > I have not check the performance implication of that with a micro
> > benchmark or the like, but I can get behind 0001 on consistency
> > grounds between the backend and
On Thu, Nov 25, 2021 at 04:04:23PM +0900, Michael Paquier wrote:
> I have not check the performance implication of that with a micro
> benchmark or the like, but I can get behind 0001 on consistency
> grounds between the backend and the frontend.
/* Now create pg_control */
InitControlFile
On Thu, Nov 25, 2021 at 10:21:40AM +0530, Amul Sul wrote:
> Thanks for the inputs -- moved timestamp setting inside update_controlfile().
I have not check the performance implication of that with a micro
benchmark or the like, but I can get behind 0001 on consistency
grounds between the backend a
On Thu, Nov 11, 2021 at 1:30 AM Bossart, Nathan wrote:
>
> On 10/1/21, 10:40 PM, "Michael Paquier" wrote:
> > On Fri, Oct 01, 2021 at 05:47:45PM +, Bossart, Nathan wrote:
> >> I'm inclined to agree that anything that calls update_controlfile()
> >> should update the timestamp.
> >
> > pg_cont
On 10/1/21, 10:40 PM, "Michael Paquier" wrote:
> On Fri, Oct 01, 2021 at 05:47:45PM +, Bossart, Nathan wrote:
>> I'm inclined to agree that anything that calls update_controlfile()
>> should update the timestamp.
>
> pg_control.h tells that:
> pg_time_t time; /* time stamp of last
On Fri, Oct 01, 2021 at 05:47:45PM +, Bossart, Nathan wrote:
> I'm inclined to agree that anything that calls update_controlfile()
> should update the timestamp.
pg_control.h tells that:
pg_time_t time; /* time stamp of last pg_control update */
So, yes, that would be more consiste
On 9/22/21, 10:03 PM, "Amul Sul" wrote:
> On Tue, Sep 21, 2021 at 9:43 PM Bossart, Nathan wrote:
>> Shouldn't we update the time in update_controlfile()?
>
> If you see the callers of update_controlfile() except for
> RewriteControlFile() no one else updates the timestamp before calling
> it, I a
On Tue, Sep 21, 2021 at 9:43 PM Bossart, Nathan wrote:
>
> On 9/20/21, 10:07 PM, "Amul Sul" wrote:
> > On Tue, Sep 21, 2021 at 4:44 AM Bossart, Nathan wrote:
> >> On 9/19/21, 11:07 PM, "Amul Sul" wrote:
> >> > I have one additional concern about the way we update the control
> >> > file, at eve
On 9/20/21, 10:07 PM, "Amul Sul" wrote:
> On Tue, Sep 21, 2021 at 4:44 AM Bossart, Nathan wrote:
>> On 9/19/21, 11:07 PM, "Amul Sul" wrote:
>> > I have one additional concern about the way we update the control
>> > file, at every place where doing the update, we need to set control
>> > file up
that, so perhaps it would be okay to move it to update_controlfile().
>
Ok, thanks, did the same in the attached version.
Regards,
Amul Sul
From c78d9abcd4a2856446d4afac240c2fcbdc0a315f Mon Sep 17 00:00:00 2001
From: Amul Sul
Date: Tue, 21 Sep 2021 00:52:55 -0400
Subject: [PATCH v3] Deduplicate code updating ControleFile's DB
On 9/19/21, 11:07 PM, "Amul Sul" wrote:
> +1, since skipping ControlFileLock for the DBState update is not the
> right thing, let's have two different functions as per your suggestion
> -- did the same in the attached version, thanks.
I see that the attached patch reorders the call to UpdateContr
. Perhaps this should be included in
> the patch set for the other thread, especially if it will need to be
> exported.
>
Ok, reverted those changes in the attached version.
I have one additional concern about the way we update the control
file, at every place where doing the update, we ne
On Thu, Sep 16, 2021 at 5:17 AM Michael Paquier wrote:
>
> On Wed, Sep 15, 2021 at 10:49:39PM +, Bossart, Nathan wrote:
> > Ah, I was missing this context. Perhaps this should be included in
> > the patch set for the other thread, especially if it will need to be
> > exported.
>
> This part o
On Wed, Sep 15, 2021 at 10:49:39PM +, Bossart, Nathan wrote:
> Ah, I was missing this context. Perhaps this should be included in
> the patch set for the other thread, especially if it will need to be
> exported.
This part of the patch is mentioned at the top of the thread:
- LWLockAcquire(
On 9/15/21, 4:47 AM, "Amul Sul" wrote:
> On Wed, Sep 15, 2021 at 12:52 AM Bossart, Nathan wrote:
>> It looks like ebdf5bf intentionally made sure that we hold
>> ControlFileLock while updating SharedRecoveryInProgress (now
>> SharedRecoveryState after 4e87c48). The thread for this change [0]
>>
On Wed, Sep 15, 2021 at 12:52 AM Bossart, Nathan wrote:
>
> On 9/13/21, 11:06 PM, "Amul Sul" wrote:
> > The patch is straightforward but the only concern is that in
> > StartupXLOG(), SharedRecoveryState now gets updated only with spin
> > lock; earlier it also had ControlFileLock in addition to
On 9/13/21, 11:06 PM, "Amul Sul" wrote:
> The patch is straightforward but the only concern is that in
> StartupXLOG(), SharedRecoveryState now gets updated only with spin
> lock; earlier it also had ControlFileLock in addition to that. AFAICU,
> I don't see any problem there, since until the star
4 Sep 2021 01:30:54 -0400
Subject: [PATCH v1] Deduplicate code updating ControleFile's DBState only.
---
src/backend/access/transam/xlog.c | 53 +--
src/include/access/xlog.h | 3 ++
2 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/src/back
23 matches
Mail list logo
