Re: Online base backup from the hot-standby

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
> --------------------------------------------
>
>
>