From: | Stephen Frost <sfrost(at)snowman(dot)net> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | David Steele <david(at)pgmasters(dot)net>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PATCH: Make pg_stop_backup() archive wait optional |
Date: | 2017-03-22 19:14:51 |
Message-ID: | 20170322191451.GY9812@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Fujii,
* Fujii Masao (masao(dot)fujii(at)gmail(dot)com) wrote:
> On Thu, Mar 23, 2017 at 12:37 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * David Steele (david(at)pgmasters(dot)net) wrote:
> >> On 3/21/17 2:34 PM, Fujii Masao wrote:
> >> >The patch basically looks good to me, but one comment is;
> >> >backup.sgml (at least the description for "Making a non-exclusive
> >> >low level backup) seems to need to be updated.
> >>
> >> Agreed. Added in the attached patch and rebased on 8027556.
>
> Thanks for updating the patch!
>
> -SELECT * FROM pg_stop_backup(false);
> +SELECT * FROM pg_stop_backup(false [, true ]);
>
> I think that it's better to get rid of "[" and "]" from the above because
> IMO this should be the command example that users actually can run.
Using the '[' and ']' are how all of the optional arguments are
specified in the documentation, see things like current_setting() in our
existing documentation:
https://www.postgresql.org/docs/9.6/static/functions-admin.html
> + If the backup process monitors the WAL archiving process independently,
> + the second parameter (which defaults to true) can be set to false to
> + prevent <function>pg_stop_backup</> from blocking until all WAL is
> + archived. Instead, the function will return as soon as the stop backup
> + record is written to the WAL. This option must be used with caution:
> + if WAL archiving is not monitored correctly then the result might be a
> + useless backup.
>
> You added this descriptions into the step #4 in the non-exclusive
> backup procedure.. But since the step #5 already explains how
> pg_stop_backup has to do with WAL archiving, I think that it's better
> to update (or add something like the above descriptions into)
> the step #5. Thought?
That seems pretty reasonable to me.
> + If the backup process monitors the WAL archiving process independently,
>
> Can we explain "monitor the WAL archiving process" part a bit more
> explicitly? For example, "monitor and ensure that all WAL segment files
> required for the backup are successfully archived".
Sure, makes sense. I'll add some language along those lines.
> > I've started looking at this. Seems pretty straight-forward and will
> > try to get it committed later today.
>
> Thanks!
My apologies if you had intended to look at committing this, I just
noticed that it hadn't been 'claimed' yet in the CF app and did so to
move forward with it. I didn't mean to step on anyone's 'toes'.
Thanks!
Stephen
From | Date | Subject | |
---|---|---|---|
Next Message | Mithun Cy | 2017-03-22 19:17:30 | Re: Patch: Write Amplification Reduction Method (WARM) |
Previous Message | Stephen Frost | 2017-03-22 19:09:57 | Re: increasing the default WAL segment size |