Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-04-04 Thread David Steele
On 4/4/17 11:42 AM, Stephen Frost wrote: > * David Steele (da...@pgmasters.net) wrote: >> On 3/22/17 4:42 PM, Peter Eisentraut wrote: >>> On 3/22/17 15:14, Stephen Frost wrote: > -SELECT * FROM pg_stop_backup(false); > +SELECT * FROM pg_stop_backup(false [, true ]); > > I think

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-04-04 Thread Stephen Frost
* David Steele (da...@pgmasters.net) wrote: > On 3/22/17 4:42 PM, Peter Eisentraut wrote: > >On 3/22/17 15:14, Stephen Frost wrote: > >>>-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

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread David Steele
On 3/22/17 4:42 PM, Peter Eisentraut wrote: On 3/22/17 15:14, Stephen Frost wrote: -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

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread Stephen Frost
* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > On 3/22/17 15:14, Stephen Frost wrote: > >> -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

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread Peter Eisentraut
On 3/22/17 15:14, Stephen Frost wrote: >> -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 '['

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread Stephen Frost
Fujii, * Fujii Masao (masao.fu...@gmail.com) wrote: > On Thu, Mar 23, 2017 at 12:37 AM, Stephen Frost 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; > >>

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread Fujii Masao
On Thu, Mar 23, 2017 at 12:37 AM, Stephen Frost wrote: > David, all, > > * 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

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-22 Thread Stephen Frost
David, all, * 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

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-21 Thread David Steele
On 3/21/17 2:34 PM, Fujii Masao wrote: On Tue, Mar 21, 2017 at 11:03 AM, Tsunakawa, Takayuki wrote: From: David Steele [mailto:da...@pgmasters.net] Well, that's embarrassing. When I recreated the function to add defaults I messed up the AS clause and did not

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-21 Thread Fujii Masao
On Tue, Mar 21, 2017 at 11:03 AM, Tsunakawa, Takayuki wrote: > From: David Steele [mailto:da...@pgmasters.net] >> Well, that's embarrassing. When I recreated the function to add defaults >> I messed up the AS clause and did not pay attention to the results of the

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-20 Thread Tsunakawa, Takayuki
From: David Steele [mailto:da...@pgmasters.net] > Well, that's embarrassing. When I recreated the function to add defaults > I messed up the AS clause and did not pay attention to the results of the > regression tests, apparently. > > Attached is a new version rebased on 88e66d1. Catalog

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-17 Thread David Steele
On 3/17/17 4:18 AM, Tsunakawa, Takayuki wrote: > From: pgsql-hackers-ow...@postgresql.org >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, >> Takayuki >> I made this ready for committer. The patch applied except for catversion.h, >> the patch content looks good, and the

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-17 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tsunakawa, > Takayuki > I made this ready for committer. The patch applied except for catversion.h, > the patch content looks good, and the target test passed as follows: Sorry, I reverted this to

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-16 Thread Tsunakawa, Takayuki
> From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier > On Fri, Mar 17, 2017 at 1:47 PM, Tsunakawa, Takayuki > wrote: > > BTW, does the developer of each feature have to modify the catalog version > in

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-16 Thread Michael Paquier
On Fri, Mar 17, 2017 at 1:47 PM, Tsunakawa, Takayuki wrote: > BTW, does the developer of each feature have to modify the catalog version in > catversion.h? It's a bit annoying to see the patch application failure on > catversion.h. Committers take care of this

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-16 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of David Steele > The attached patch udpates the docs per your suggestion and has been rebased > on master at d69fae2. I made this ready for committer. The patch applied except for catversion.h, the

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-13 Thread David Steele
Hi Robert, On 3/6/17 12:48 PM, Robert Haas wrote: > On Sat, Mar 4, 2017 at 9:12 AM, David Steele wrote: >> Yes, that makes sense. Attached are two patches as requested: >> >> 01 - Just marks pg_stop_backup() variants as parallel restricted >> 02 - Add the wait_for_archive

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-06 Thread Robert Haas
On Mon, Mar 6, 2017 at 12:53 PM, Stephen Frost wrote: > Regarding 02, I certainly see that as valuable for the reasons which > David outlined in his initial email. I can certainly take point on > getting it committed, but I wouldn't complain if someone else does > either.

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-06 Thread Stephen Frost
Robert, all, * Robert Haas (robertmh...@gmail.com) wrote: > On Sat, Mar 4, 2017 at 9:12 AM, David Steele wrote: > > Yes, that makes sense. Attached are two patches as requested: > > > > 01 - Just marks pg_stop_backup() variants as parallel restricted > > 02 - Add the

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-06 Thread Robert Haas
On Sat, Mar 4, 2017 at 9:12 AM, David Steele wrote: > Yes, that makes sense. Attached are two patches as requested: > > 01 - Just marks pg_stop_backup() variants as parallel restricted > 02 - Add the wait_for_archive param to pg_stop_backup(). > > These apply cleanly on

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-04 Thread David Steele
Hi Robert, On 3/4/17 1:58 AM, Robert Haas wrote: > On Wed, Mar 1, 2017 at 9:07 AM, David Steele wrote: >> On 2/28/17 10:22 PM, Robert Haas wrote: >>> On Tue, Feb 28, 2017 at 6:22 AM, David Steele wrote: >> I'm not sure that's the case. It seems

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-03 Thread Robert Haas
On Wed, Mar 1, 2017 at 9:07 AM, David Steele wrote: > On 2/28/17 10:22 PM, Robert Haas wrote: >> On Tue, Feb 28, 2017 at 6:22 AM, David Steele wrote: > I'm not sure that's the case. It seems like it should lock just as > multiple backends would

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-01 Thread David Steele
On 3/1/17 5:11 PM, Jim Nasby wrote: > On 2/27/17 6:25 PM, David Steele wrote: >> The purpose of this patch is to make waiting for archive optional, with >> the default being the current behavior, i.e., to wait for all WAL to be >> archived. This functionality is already used internally by >>

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-03-01 Thread Jim Nasby
On 2/27/17 6:25 PM, David Steele wrote: The purpose of this patch is to make waiting for archive optional, with the default being the current behavior, i.e., to wait for all WAL to be archived. This functionality is already used internally by pg_basebackup, so the only real change is to expose

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-28 Thread David Steele
On 2/28/17 10:22 PM, Robert Haas wrote: > On Tue, Feb 28, 2017 at 6:22 AM, David Steele wrote: I'm not sure that's the case. It seems like it should lock just as multiple backends would now. One process would succeed and the others would error. Maybe I'm

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-28 Thread Robert Haas
On Tue, Feb 28, 2017 at 6:22 AM, David Steele wrote: >>> I'm not sure that's the case. It seems like it should lock just as >>> multiple backends would now. One process would succeed and the others >>> would error. Maybe I'm missing something? >> >> Hm, any errors

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-27 Thread David Steele
On 2/27/17 7:50 PM, Michael Paquier wrote: > On Tue, Feb 28, 2017 at 9:42 AM, David Steele wrote: >> On 2/27/17 7:38 PM, Michael Paquier wrote: >>> On Tue, Feb 28, 2017 at 9:25 AM, David Steele wrote: I also marked the pg_stop_* functions as

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-27 Thread Michael Paquier
On Tue, Feb 28, 2017 at 9:42 AM, David Steele wrote: > On 2/27/17 7:38 PM, Michael Paquier wrote: >> On Tue, Feb 28, 2017 at 9:25 AM, David Steele wrote: >>> I also marked the pg_stop_* functions as parallel restricted, the same >>> as pg_start_backup().

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-27 Thread David Steele
On 2/27/17 7:38 PM, Michael Paquier wrote: > On Tue, Feb 28, 2017 at 9:25 AM, David Steele wrote: >> I also marked the pg_stop_* functions as parallel restricted, the same >> as pg_start_backup(). Previously they were parallel safe which I don't >> believe is accurate for

Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-27 Thread Michael Paquier
On Tue, Feb 28, 2017 at 9:25 AM, David Steele wrote: > I also marked the pg_stop_* functions as parallel restricted, the same > as pg_start_backup(). Previously they were parallel safe which I don't > believe is accurate for the non-exclusive version at the very least, >

[HACKERS] PATCH: Make pg_stop_backup() archive wait optional

2017-02-27 Thread David Steele
Currently pg_stop_backup() will wait for all WAL to be archived before returning. If there is a problem with the archive command or a large backlog it may not return for a very long time (if ever). Backup software is faced with the choice of cancelling the query and hoping the stop backup record