Re: Online base backup from the hot-standby

On 11-10-31 12:11 AM, Jun Ishiduka wrote:
>>
>> Agreed. I'll extract FPW stuff from the patch that I submitted, and revise it
>> as the infrastructure patch.
>>
>> The changes of pg_start_backup() etc that Ishiduka-san did are also
>> a server-side infrastructure. I will extract them as another infrastructure one.
>>
>> Ishiduka-san, if you have time, feel free to try the above, barring objection.
>
> Done.
> Changed the name of the patch.
>
> <Modifications>
> So changed to the positioning of infrastructure,
> * Removed the documentation.
> * changed to an error when you run pg_start/stop_backup() on the standby.
>
>

Here is my stab at reviewing this version of this version of the patch.

Submission
-------------------
The purpose of this version of the patch is to provide some
infrastructure needed for backups from the slave without having to solve
some of the usability issues raised in previous versions of the patch.

This patch applied fine earlier versions of head but it doesn't today.
Simon moved some of the code touched by this patch as part of the xlog
refactoring. Please post an updated/rebased version of the patch.

I think the purpose of this patch is to provide

a) The code changes to record changes to fpw state of the master in WAL.
b) Track the state of FPW while in recovery mode

This version of the patch is NOT intended to allow SQL calls to
pg_start_backup() on slaves to work. This patch lays the infrastructure
for another patch (which I haven't seen) to allow pg_basebackup to do a
base backup from a slave assuming fpw=on has been set on the master (my
understanding of this patch is that it puts into place all of the pieces
required for the pg_basebackup patch to detect if fpw!=on and abort).

The consensus upthread was to get this infrastructure in and figure out
a safe+usable way of doing a slave backup without pg_basebackup later.

The patch seems to do what I expect of it.

I don't see any issues with most of the code changes in this patch.
However I admit that even after reviewing many versions of this patch I
still am not familiar enough with the recovery code to comment on a lot
of the details.

One thing I did see:

In pg_ctl.c

! if (stat(recovery_file, &statbuf) != 0)
! print_msg(_("WARNING: online backup mode is active\n"
! "Shutdown will not complete until pg_stop_backup() is called.\n\n"));
! else
! print_msg(_("WARNING: online backup mode is active if you can connect
as a superuser to server\n"
! "If so, shutdown will not complete until pg_stop_backup() is
called.\n\n"));

I am having difficulty understanding what this error message is trying
to tell me. I think it is telling me (based on the code comments) that
if I can't connect to the server because the server is not yet accepting
connections then I shouldn't worry about anything. However if the server
is accepting connections then I need to login and call pg_stop_backup().

Maybe
"WARNING: online backup mode is active. If your server is accepting
connections then you must connect as superuser and run pg_stop_backup()
before shutdown will complete"

I will wait on attempting to test the patch until you have sent a
version that applies against the current HEAD.

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