Re: Online base backup from the hot-standby

Lists: pgsql-hackers
From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Online base backup from the hot-standby
Date: 2011-06-14 06:03:47
Message-ID: 201106140603.p5E63c2F029162@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

( Quotation from
http://archives.postgresql.org/pgsql-hackers/2011-05/msg01396.php )

> STEP1: Make startup process to acquire backup-end-position from
> not only backup-end record but also backup-history-file .
> * startup process allows to acquire backup-end-position
> from backup-history-file .

I have created a patch to the above-mentioned content.

Please check it.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------

Attachment Content-Type Size
standby_online_backup_01.patch application/octet-stream 12.0 KB

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-14 06:30:20
Message-ID: 4DF6FFFC.3040003@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 14.06.2011 09:03, Jun Ishiduka wrote:
> ( Quotation from
> http://archives.postgresql.org/pgsql-hackers/2011-05/msg01396.php )
>
>> STEP1: Make startup process to acquire backup-end-position from
>> not only backup-end record but also backup-history-file .
>> * startup process allows to acquire backup-end-position
>> from backup-history-file .
>
>
> I have created a patch to the above-mentioned content.

I still think that's headed in the wrong direction.
(http://archives.postgresql.org/pgsql-hackers/2011-05/msg01405.php)

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: heikki(dot)linnakangas(at)enterprisedb(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-14 06:52:12
Message-ID: 201106140652.p5E6q441022089@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> I still think that's headed in the wrong direction.
> (http://archives.postgresql.org/pgsql-hackers/2011-05/msg01405.php)

Please check these mails, and teach the reason for content of the wrong
direction.
(http://archives.postgresql.org/pgsql-hackers/2011-06/msg00209.php)
(http://archives.postgresql.org/pgsql-hackers/2011-05/msg01566.php)

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-21 21:20:01
Message-ID: BLU0-SMTP948D325D1B6471C1C61BF98E510@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11-06-14 02:52 AM, Jun Ishiduka wrote:
>> I still think that's headed in the wrong direction.
>> (http://archives.postgresql.org/pgsql-hackers/2011-05/msg01405.php)
> Please check these mails, and teach the reason for content of the wrong
> direction.
> (http://archives.postgresql.org/pgsql-hackers/2011-06/msg00209.php)
> (http://archives.postgresql.org/pgsql-hackers/2011-05/msg01566.php)
>
>

Jun, I've been reviewing these threads as a start to reviewing your
patch (I haven't yet looked at the patch).

I *think* the concern is that

1) Today you can do a backup by just calling pg_start_backup('x'); copy
the data directory and
pg_stop_backup(); You do not need to use pg_basebackup to create a
backup. The solution you are proposing would require pg_basebackup to be
used to build backups from standby servers.

2) If I run pg_basebackup but do not specify '-x' then no pg_xlog
segments are included in the output. The relevant pg_xlog segments are
in the archive from the master. I can see situations where you are
already copying the archive to the remote site that the new standby will
be created in so you don't want to have to copy the pg_xlog segments
twice over your network.

What Heikki is proposing will work both when you aren't using
pg_basebackup (as long the output of pg_stop_backup() is somehow
captured in a way that it can be read) and will also work with
pg_basebackup when '-x' isn't specified.

Steve

> --------------------------------------------
> Jun Ishizuka
> NTT Software Corporation
> TEL:045-317-7018
> E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
> --------------------------------------------
>
>
>


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: Steve Singer <ssinger_pg(at)sympatico(dot)ca>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-23 06:41:09
Message-ID: 201106230641.p5N6f7Zn029229@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> 1) Today you can do a backup by just calling pg_start_backup('x'); copy
> the data directory and
> pg_stop_backup(); You do not need to use pg_basebackup to create a
> backup. The solution you are proposing would require pg_basebackup to be
> used to build backups from standby servers.

YES.

> 2) If I run pg_basebackup but do not specify '-x' then no pg_xlog
> segments are included in the output. The relevant pg_xlog segments are
> in the archive from the master. I can see situations where you are
> already copying the archive to the remote site that the new standby will
> be created in so you don't want to have to copy the pg_xlog segments
> twice over your network.

No, I don't matter because of the same behavior as 9.1.
Please see the part of "Before:" of the following answer.

> What Heikki is proposing will work both when you aren't using
> pg_basebackup (as long the output of pg_stop_backup() is somehow
> captured in a way that it can be read) and will also work with
> pg_basebackup when '-x' isn't specified.

I receive this mail, so I notice I do wrong recognition to what
Heikki is proposing.

my recognition:
Before:
* I thought Heikki proposes, "Execute SQL(pg_start_backup('x'); copy
the data directory and pg_stop_backup();) from the standby server
to the primary server".
-> I disliked this.
Now:
* Heikki is proposing both No using pg_basebackup and Not specify -x.
So,
* Use the output of pg_stop_backup().
* Don't use backup history file.
he thinks.

Right?

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-23 14:21:48
Message-ID: BLU0-SMTP540DC369919CCA08DD2D368E530@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11-06-23 02:41 AM, Jun Ishiduka wrote:
> I receive this mail, so I notice I do wrong recognition to what
> Heikki is proposing.
>
> my recognition:
> Before:
> * I thought Heikki proposes, "Execute SQL(pg_start_backup('x'); copy
> the data directory and pg_stop_backup();) from the standby server
> to the primary server".
> -> I disliked this.
> Now:
> * Heikki is proposing both No using pg_basebackup and Not specify -x.
> So,
> * Use the output of pg_stop_backup().
> * Don't use backup history file.
> he thinks.
>
> Right?
>

What I think he is proposing would not require using pg_stop_backup()
but you could also modify pg_stop_back() to work as well.

What do you think of this idea?

Do you see how the patch can be reworked to accomplish this?

> --------------------------------------------
> Jun Ishizuka
> NTT Software Corporation
> TEL:045-317-7018
> E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
> --------------------------------------------
>
>
>


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: ssinger_pg(at)sympatico(dot)ca
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-24 04:41:42
Message-ID: 201106240441.p5O4fe9J005881@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> What I think he is proposing would not require using pg_stop_backup()
> but you could also modify pg_stop_back() to work as well.
>
> What do you think of this idea?
>
> Do you see how the patch can be reworked to accomplish this?

The logic that not use pg_stop_backup() would be difficult,
because pg_stop_backup() is used to identify minRecoveryPoint.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-24 12:32:03
Message-ID: BLU0-SMTP695129D4A93A996B7621B28E520@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11-06-24 12:41 AM, Jun Ishiduka wrote:
>
> The logic that not use pg_stop_backup() would be difficult,
> because pg_stop_backup() is used to identify minRecoveryPoint.
>

Considering everything that has been discussed on this thread so far.

Do you still think your patch is the best way to accomplish base backups
from standby servers?
If not what changes do you think should be made?

Steve

> --------------------------------------------
> Jun Ishizuka
> NTT Software Corporation
> TEL:045-317-7018
> E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
> --------------------------------------------
>
>
>


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: ssinger_pg(at)sympatico(dot)ca
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-28 05:52:52
Message-ID: 201106280552.p5S5qsSi025889@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> Considering everything that has been discussed on this thread so far.
>
> Do you still think your patch is the best way to accomplish base backups
> from standby servers?
> If not what changes do you think should be made?

I reconsider the way to not use pg_stop_backup().

Process of online base backup on standby server:
1. pg_start_backup('x');
2. copy the data directory
3. copy *pg_control*

Behavior while restore:
* read "Minimum recovery ending location" of the copied pg_control.
* use the value with the same purposes as the end-of-backup location.
-> When the value is equal to 0/0, this behavior do not do.
This situation is to acquire backup from master server.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-29 02:39:01
Message-ID: BLU0-SMTP292DD46F97A00D91E9598C8E590@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11-06-28 01:52 AM, Jun Ishiduka wrote:
>> Considering everything that has been discussed on this thread so far.
>>
>> Do you still think your patch is the best way to accomplish base backups
>> from standby servers?
>> If not what changes do you think should be made?
> I reconsider the way to not use pg_stop_backup().
>
> Process of online base backup on standby server:
> 1. pg_start_backup('x');
> 2. copy the data directory
> 3. copy *pg_control*
>
> Behavior while restore:
> * read "Minimum recovery ending location" of the copied pg_control.
> * use the value with the same purposes as the end-of-backup location.
> -> When the value is equal to 0/0, this behavior do not do.
> This situation is to acquire backup from master server.
>

The behaviour you describe above sounds okay to me, if someone else sees
issues with this then they should speak up (ideally before you go off
and write a new patch)

I'm going to consolidate my other comments below so this can act as a
more complete review.

Usability Review
-----------------
We would like to be able to perform base backups from slaves without
having to call pg_start_backup() on the master. We can not currently do
this. The patch tries to do this. From a useability point of view it
would be nice if this could be done both manually with pg_start_backup()
and with pg_basebackup.

The main issue I have with the first patch you submitted is that it does
not work for cases where you don't want to call pg_basebackup or you
don't want to include the wal segments in the output of pg_basebackup.
There are a number of these use-cases (examples include the wal is
already available on an archive server, or you want to use
filesystem/disk array level snapshots instead of tar) . I feel your
above proposal to copy the control file as the last step in the
basebackup and the get the minRecoveryEnding location from this solves
these issues. It would be nice if things 'worked' when calling
pg_basebackup against the slave (maybe by having perform_base_backup()
resend the control file after it has sent the log?).

Feature test & Performance review
-----------------
Skipped since a new patch is coming

Coding Review
------------------
I didn't look too closely at the code since a new patch that might
change a lot of the code. I did like how you added comments to most of
the larger code blocks that you added.

Architecture Review
-----------------------
There were some concerns with your original approach but using the
control file was suggested by Heikki and seems sound to me.

I'm marking this 'waiting for author' , if you don't think you'll be
able to get a reworked patch out during this commitfest then you should
move it to 'returned with feedback'

Steve

> --------------------------------------------
> Jun Ishizuka
> NTT Software Corporation
> TEL:045-317-7018
> E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
> --------------------------------------------
>
>
>


From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: ssinger_pg(at)sympatico(dot)ca, heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-30 03:58:50
Message-ID: BANLkTi=iiYWYmq-pt-zgaGXS_FQt6VngUA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/6/28 Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>:
>
>> Considering everything that has been discussed on this thread so far.
>>
>> Do you still think your patch is the best way to accomplish base backups
>> from standby servers?
>> If not what changes do you think should be made?
>
> I reconsider the way to not use pg_stop_backup().
>
> Process of online base backup on standby server:
>  1. pg_start_backup('x');
>  2. copy the data directory
>  3. copy *pg_control*

Who deletes the backup_label file created by pg_start_backup()?
Isn't pg_stop_backup() required to do that?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org, ssinger_pg(at)sympatico(dot)ca, heikki(dot)linnakangas(at)enterprisedb(dot)com
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-30 04:06:44
Message-ID: BANLkTina+45uuMoJiEwxd6mUgWpbDG2HDw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jun 30, 2011 5:59 AM, "Fujii Masao" <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> 2011/6/28 Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>:
> >
> >> Considering everything that has been discussed on this thread so far.
> >>
> >> Do you still think your patch is the best way to accomplish base
backups
> >> from standby servers?
> >> If not what changes do you think should be made?
> >
> > I reconsider the way to not use pg_stop_backup().
> >
> > Process of online base backup on standby server:
> > 1. pg_start_backup('x');
> > 2. copy the data directory
> > 3. copy *pg_control*
>
> Who deletes the backup_label file created by pg_start_backup()?
> Isn't pg_stop_backup() required to do that?

You need it to take the system out of backup mode as well, don't you?

/Magnus


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: magnus(at)hagander(dot)ne, masao(dot)fujii(at)gmail(dot)com, ssinger_pg(at)sympatico(dot)ca
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-06-30 06:38:45
Message-ID: 201106300638.p5U6cnoL014540@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> > > Process of online base backup on standby server:
> > > 1. pg_start_backup('x');
> > > 2. copy the data directory
> > > 3. copy *pg_control*
> >
> > Who deletes the backup_label file created by pg_start_backup()?
> > Isn't pg_stop_backup() required to do that?
>
> You need it to take the system out of backup mode as well, don't you?

Certainly so.

Add to the above process:
4. pg_stop_backup();

But I do not consider a case such as to promote in backup mode yet.
I need to think a little, including it.

On this commitfest, the goal of the patch is to be able to be
recovered using "Minimum recovery ending location" in the control file.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: magnus(at)hagander(dot)net
Cc: heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-01 05:04:27
Message-ID: 201107010504.p6154Vo0012666@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> On this commitfest, the goal of the patch is to be able to be
> recovered using "Minimum recovery ending location" in the control file.

Done.

Regards.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------

Attachment Content-Type Size
standby_online_backup_02.patch application/octet-stream 2.6 KB

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org, Steve Singer <ssinger_pg(at)sympatico(dot)ca>
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-04 03:19:16
Message-ID: CAHGQGwF2hwT1_xRGBkB-UPXw8qUgc=9napx5Q+u35GH9-qwDWA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/7/1 Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>:
>
>> On this commitfest, the goal of the patch is to be able to be
>> recovered using "Minimum recovery ending location" in the control file.
>
> Done.

When the standby restarts after it crashes during recovery, it always
checks whether recovery has reached the backup end location by
using minRecoveryPoint even though the standby doesn't start from
the backup. This looks odd.

- XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
+ (XLogRecPtrIsInvalid(ControlFile->backupStartPoint) ||
+ reachedControlMinRecoveryPoint == true))

The flag 'reachedControlMinRecoveryPoint' is really required? When recovery
reaches minRecoveryPoint, ControlFile->backupStartPoint is reset to zero. So
we can check whether recovery has reached minRecoveryPoint or not by only
doing XLogRecPtrIsInvalid(ControlFile->backupStartPoint). No?

We should check if recovery has reached minRecoveryPoint before calling
CheckRecoveryConsistency() after reading new WAL record? Otherwise,
even if recovery has reached minRecoveryPoint, the standby cannot think
that it's in consistent state until it reads new WAL record.

+ if (XLByteLT(ControlFile->minRecoveryPoint, EndRecPtr))
+ ControlFile->minRecoveryPoint = EndRecPtr;

Why does ControlFile->minRecoveryPoint need to be set to EndRecPtr?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: masao(dot)fujii(at)gmail(dot)com
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, ssinger_pg(at)sympatico(dot)ca, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-04 09:25:12
Message-ID: 201107040925.p649PIRn024250@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> When the standby restarts after it crashes during recovery, it always
> checks whether recovery has reached the backup end location by
> using minRecoveryPoint even though the standby doesn't start from
> the backup. This looks odd.

Certainly.

But, in this case, the state before recovery starts is lost.
Therefore, postgres can not see that the backup got from whether
standby server or master.

What should?
Should use pg_control?
Ex.
* Add 'Where to get backup' to pg_control. (default 'none')
* When recovery starts, it checks it whether 'none'.
* When minRecoveryPoint equals 0/0, change 'master'.
* When minRecoveryPoint do not equals 0/0, change 'slave'.
* When it reached the end of recovery, change 'none' .

> - XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
> + (XLogRecPtrIsInvalid(ControlFile->backupStartPoint) ||
> + reachedControlMinRecoveryPoint == true))

> The flag 'reachedControlMinRecoveryPoint' is really required? When recovery
> reaches minRecoveryPoint, ControlFile->backupStartPoint is reset to zero. So
> we can check whether recovery has reached minRecoveryPoint or not by only
> doing XLogRecPtrIsInvalid(ControlFile->backupStartPoint). No?

Yes.
'reachedControlMinRecoveryPoint' is unnecessary.

> We should check if recovery has reached minRecoveryPoint before calling
> CheckRecoveryConsistency() after reading new WAL record? Otherwise,
> even if recovery has reached minRecoveryPoint, the standby cannot think
> that it's in consistent state until it reads new WAL record.

This is a same sequence with a case of backup end location.
It should be no changed.

> + if (XLByteLT(ControlFile->minRecoveryPoint, EndRecPtr))
> + ControlFile->minRecoveryPoint = EndRecPtr;

> Why does ControlFile->minRecoveryPoint need to be set to EndRecPtr?

Yes.
I delete it.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, ssinger_pg(at)sympatico(dot)ca, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-05 05:54:18
Message-ID: CAHGQGwGHUV4C99CC1xddcStJNQ9gb-tHg5J7+o1q+gAabt27Vw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/7/4 Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>:
>
>> When the standby restarts after it crashes during recovery, it always
>> checks whether recovery has reached the backup end location by
>> using minRecoveryPoint even though the standby doesn't start from
>> the backup. This looks odd.
>
> Certainly.
>
> But, in this case, the state before recovery starts is lost.
> Therefore, postgres can not see that the backup got from whether
> standby server or master.
>
> What should?
> Should use pg_control?
>  Ex.
>   * Add 'Where to get backup' to pg_control. (default 'none')
>   * When recovery starts, it checks it whether 'none'.
>      * When minRecoveryPoint equals 0/0, change 'master'.
>      * When minRecoveryPoint do not equals 0/0, change 'slave'.
>   * When it reached the end of recovery, change 'none' .

What about using backupStartPoint to check whether this recovery
started from the backup or not?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: masao(dot)fujii(at)gmail(dot)com
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, ssinger_pg(at)sympatico(dot)ca, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-05 08:41:33
Message-ID: 201107050842.p658g1s0013581@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> What about using backupStartPoint to check whether this recovery
> started from the backup or not?

No, postgres can check whether this recovery started from the backup
or not, but can not check whether standby server or master (got backup
from).

Once recovery started, backupStartPoint is recorded to pg_control until
recovery reaches backup end location, it is not related to any backup
server.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------


From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, ssinger_pg(at)sympatico(dot)ca, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-05 10:29:05
Message-ID: CAHGQGwEHTNLyhDL2H-gyyoZ8kpPC7zSM9um0X9sbo0iNhJ0=eg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2011/7/5 Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>:
>
>> What about using backupStartPoint to check whether this recovery
>> started from the backup or not?
>
> No, postgres can check whether this recovery started from the backup
> or not, but can not check whether standby server or master (got backup
> from).

Oh, right. We cannot distinguish the following two cases just by using
minRecoveryPoint and backupStartPoint.

* The standby starts from the backup taken from the standby
* The standby starts after it crashes during recovering from the
backup taken from the master

As you proposed, adding new field which stores the backup end location
taken from minRecoveryPoint, into pg_control sounds good idea.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: masao(dot)fujii(at)gmail(dot)com
Cc: magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, ssinger_pg(at)sympatico(dot)ca, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-08 01:22:20
Message-ID: 201107080122.p681MTl2009948@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> As you proposed, adding new field which stores the backup end location
> taken from minRecoveryPoint, into pg_control sounds good idea.

Update patch.

Regards.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------

Attachment Content-Type Size
standby_online_backup_03.patch application/octet-stream 4.9 KB

From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
Cc: masao(dot)fujii(at)gmail(dot)com, magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-10 16:20:39
Message-ID: BLU0-SMTP55C661AD49484CEECF665D8E420@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11-07-07 09:22 PM, Jun Ishiduka wrote:
>> As you proposed, adding new field which stores the backup end location
>> taken from minRecoveryPoint, into pg_control sounds good idea.
> Update patch.
>
Here is a review of the updated patch

This version of the patch adds a field into pg_controldata that tries to
store the source of the base backup while in recovery mode.
I think your ultimate goal with this patch is to be able to take a
backup of a running hot-standby slave and recover it as another
instance. This patch seems to provide the ability to have the second
slave stop recovery at minRecoveryPoint from the control file.

My understanding of the procedure you want to get to to take base
backups off a slave is

1. execute pg_start_backup('x') on the slave (*)
2. take a backup of the data dir
3. call pg_stop_backup() on the slave
4. Copy the control file on the slave

This patch only addresses the recovery portions.

* - I think your goal is to be able to run pg_start_backup() on the
slave, the patch so far doesn't do this. If your goal was require this
to be run on the master, then correct me.

Code Review
-------------------
A few comments on the code

> *** postgresql/src/include/catalog/pg_control.h 2011-06-30
> 10:04:48.000000000 +0900
> --- postgresql_with_patch/src/include/catalog/pg_control.h 2011-07-07
> 18:23:56.000000000 +0900
> ***************
> *** 142,147 ****
> --- 142,157 ----
> XLogRecPtr backupStartPoint;
>
> /*
> + * Postgres keeps where to take a backup server.
> + *
> + * backupserver is "none" , "master" or "slave", its default is "none".
> + * When postgres starts and it is "none", it is updated to either
> "master"
> + * or "slave" with minRecoveryPoint of the backup server.
> + * When postgres reaches backup end location, it is updated to "none".
> + */
> + int backupserver;
> +
> + /*
> * Parameter settings that determine if the WAL can be used for archival
> * or hot standby.
> */

I don't think the above comment is very clear on what backupserver is.
Perhaps

/**
* backupserver is used while postgresql is in recovery mode to
* store the location of where the backup comes from.
* When Postgres starts recovery operations
* it is set to "none". During recovery it is updated to either "master",
or "slave"
* When recovery operations finish it is updated back to "none"
**/

Also shouldn't backupServer be the enum type of 'BackupServer' not int?
Other enums in the structure such as DBState are defined this way.

Testing Review
----------------------

Since I can't yet call pg_start_backup or pg_stop_backup() on the slave
I am calling them on the master.
(I also did some testing where I didn't put the system into backup
mode). I admit that I am not sure what to look for as an indication that
the system isn't recovering to the correct point. In much of my testing
I was just verifying that the slave started and my data 'looked' okay.

I seem to get this warning in my logs when I start up the instance based
on the slave backup.
LOG: 00000: database system was interrupted while in recovery at log
time 2011-07-08 18:40:20 EDT
HINT: If this has occurred more than once some data might be corrupted
and you might need to choose an earlier recovery target

I'm wondering if this warning is a bit misleading to users because it is
an expected message when starting up an instance based on a slave backup
(because the slave was already in recovery mode). If I shutdown this
instance and start it up again I keep getting the warning. My
understanding of your patch is that there shouldn't be any risk of
corruption in that case (assuming your patch has no bugs). Can/should we
be suppressing this message when we detect that we are recovering from a
slave backup?

The direction of the patch has changed a bit during this commit fest. I
think it would be good to provide an update on the rest of the changes
you plan for this to be a complete useable feature. That would make it
easier to comment on something you
missed versus something your planning on dealing with in the next stage.

Steve

> Regards.
>
> --------------------------------------------
> Jun Ishizuka
> NTT Software Corporation
> TEL:045-317-7018
> E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
> --------------------------------------------
>
>
>


From: Jun Ishiduka <ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp>
To: ssinger_pg(at)sympatico(dot)ca
Cc: masao(dot)fujii(at)gmail(dot)com, magnus(at)hagander(dot)net, heikki(dot)linnakangas(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Online base backup from the hot-standby
Date: 2011-07-12 07:12:44
Message-ID: 201107120712.p6C7Cuv3032235@ccmds32.silk.ntts.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> This version of the patch adds a field into pg_controldata that tries to
> store the source of the base backup while in recovery mode.
> I think your ultimate goal with this patch is to be able to take a
> backup of a running hot-standby slave and recover it as another
> instance. This patch seems to provide the ability to have the second
> slave stop recovery at minRecoveryPoint from the control file.
>
>
> My understanding of the procedure you want to get to to take base
> backups off a slave is
>
> 1. execute pg_start_backup('x') on the slave (*)
> 2. take a backup of the data dir
> 3. call pg_stop_backup() on the slave
> 4. Copy the control file on the slave
>
> This patch only addresses the recovery portions.

Yes.

> I don't think the above comment is very clear on what backupserver is.
> Perhaps
>
> /**
> * backupserver is used while postgresql is in recovery mode to
> * store the location of where the backup comes from.
> * When Postgres starts recovery operations
> * it is set to "none". During recovery it is updated to either "master",
> or "slave"
> * When recovery operations finish it is updated back to "none"
> **/

Done.

> Also shouldn't backupServer be the enum type of 'BackupServer' not int?
> Other enums in the structure such as DBState are defined this way.

Now, this is a same as wal_level, not DBState. No?

> Since I can't yet call pg_start_backup or pg_stop_backup() on the slave
> I am calling them on the master.
> (I also did some testing where I didn't put the system into backup
> mode). I admit that I am not sure what to look for as an indication that
> the system isn't recovering to the correct point. In much of my testing
> I was just verifying that the slave started and my data 'looked' okay.

Updated patch as can execute pg_start/stop_backup() on standby server.
One-pass of above steps(from 1. to 4.) is now done on this.
However, there are conditions.
* Master's full_page_write = on.
* On the slave, do not execute stop/promote operation before pg_stop_backup() is executed.
* the result of pg_start_backup() may exceed the result of pg_stop_backup().

> I seem to get this warning in my logs when I start up the instance based
> on the slave backup.
> LOG: 00000: database system was interrupted while in recovery at log
> time 2011-07-08 18:40:20 EDT
> HINT: If this has occurred more than once some data might be corrupted
> and you might need to choose an earlier recovery target
>
> I'm wondering if this warning is a bit misleading to users because it is
> an expected message when starting up an instance based on a slave backup
> (because the slave was already in recovery mode). If I shutdown this
> instance and start it up again I keep getting the warning. My
> understanding of your patch is that there shouldn't be any risk of
> corruption in that case (assuming your patch has no bugs). Can/should we
> be suppressing this message when we detect that we are recovering from a
> slave backup?

This has not been supported yet.
I do not see what state of this message.

Always happens when backup is taken from slave.
What do you think about an approach to add context, "unless take backup from slave"?

> The direction of the patch has changed a bit during this commit fest. I
> think it would be good to provide an update on the rest of the changes
> you plan for this to be a complete useable feature. That would make it
> easier to comment on something you
> missed versus something your planning on dealing with in the next stage.

I see.

I will provide a patch which can exeute pg_start/stop_backup
including to solve above comment and conditions in next stage.
Then please review.

I change this patch status to "Returned with feedback".

Regards.

--------------------------------------------
Jun Ishizuka
NTT Software Corporation
TEL:045-317-7018
E-Mail: ishizuka(dot)jun(at)po(dot)ntts(dot)co(dot)jp
--------------------------------------------

Attachment Content-Type Size
standby_online_backup_04.patch application/octet-stream 8.3 KB