Fujii, * Fujii Masao (masao.fu...@gmail.com) wrote: > On Thu, Mar 23, 2017 at 12:37 AM, Stephen Frost <sfr...@snowman.net> wrote: > > * David Steele (da...@pgmasters.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
signature.asc
Description: Digital signature
