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
On Tue, Sep 21, 2021 at 4:44 AM Bossart, Nathan wrote:
>
> 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, thank
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
On Thu, Sep 16, 2021 at 4:19 AM Bossart, Nathan wrote:
>
> 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
> >
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
22 matches
Mail list logo
