Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-28 Thread Peter Eisentraut
On 27/11/2018 15:16, Sergei Kornilov wrote: > Hi > >> The attached seems to be the simplest way to fix this. (Needs >> documentation updates, test updates, error message refinement.) > > Thank you! > I add documentation change, tests and rephrased error message. I have committed it along these

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-27 Thread Sergei Kornilov
Hi > The attached seems to be the simplest way to fix this. (Needs > documentation updates, test updates, error message refinement.) Thank you! I add documentation change, tests and rephrased error message. regards, Sergeidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-27 Thread Peter Eisentraut
On 27/11/2018 13:29, Peter Eisentraut wrote: > On 27/11/2018 10:10, Sergei Kornilov wrote: >> Hello >>  - recovery_target = immediate was replaced with recovery_target_immediate bool GUC >>> >>> Why? >> Due this comment: >>

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-27 Thread Peter Eisentraut
On 27/11/2018 10:10, Sergei Kornilov wrote: > Hello > >>>  - recovery_target = immediate was replaced with recovery_target_immediate >>> bool GUC >> >> Why? > Due this comment: > https://www.postgresql.org/message-id/20181126172118.GY3415%40tamriel.snowman.net >> I've not been following this

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-27 Thread Sergei Kornilov
Hello >>  - recovery_target = immediate was replaced with recovery_target_immediate >> bool GUC > > Why? Due this comment: https://www.postgresql.org/message-id/20181126172118.GY3415%40tamriel.snowman.net > I've not been following this very closely, but seems like > recovery_target_string is a

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-27 Thread Peter Eisentraut
On 26/11/2018 21:30, Sergei Kornilov wrote: > - recovery_target = immediate was replaced with recovery_target_immediate > bool GUC Why? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, * Sergei Kornilov (s...@zsrv.org) wrote: > > I'd encourage you to look through the diff after you're finished hacking > > before sending it to the list, in case things get left in that should be > > removed, as below... > I am so sorry. I have a look before sending, but... > It's night

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hi > I'd encourage you to look through the diff after you're finished hacking > before sending it to the list, in case things get left in that should be > removed, as below... I am so sorry. I have a look before sending, but... It's night in my timezone. I will fix tomorrow. >>  + if

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, * Sergei Kornilov (s...@zsrv.org) wrote: > Updated patch attached: > - added testcase to verify database does not start with multiple recovery > targets > - recovery_target = immediate was replaced with recovery_target_immediate > bool GUC I'd encourage you to look through the diff

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hello Updated patch attached: - added testcase to verify database does not start with multiple recovery targets - recovery_target = immediate was replaced with recovery_target_immediate bool GUC thank you! regards, Sergeidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread David Steele
On 11/26/18 1:15 PM, Andres Freund wrote: > On 2018-11-26 15:09:43 -0300, Alvaro Herrera wrote: >> On 2018-Nov-26, Stephen Frost wrote: >> >>> I would think we'd have the different GUCs and then the check functions >>> would only validate that they're valid inputs and then when we get to >>> the

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, * Sergei Kornilov (s...@zsrv.org) wrote: > > I think I would have done 'if (targetSettingsCount != 1)' here. > Streaming replication does not have any recovery_target. > Well, i can check StandbyModeRequested too and allow 0 recovery_target only > for standby mode. Ah. Agreed. > >>

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hello > I think I would have done 'if (targetSettingsCount != 1)' here. Streaming replication does not have any recovery_target. Well, i can check StandbyModeRequested too and allow 0 recovery_target only for standby mode. >>  -# Multiple targets >>  -# Last entry has priority (note that an

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Andres Freund
On 2018-11-26 13:30:18 -0500, Stephen Frost wrote: > Well, we could start with: that isn’t how things work today, nor how it > used to work before this patch, and we’ve not had anyone asking for it > except for people on this thread making things up. Thanks for assuming good faith.

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, On Mon, Nov 26, 2018 at 13:15 Andres Freund wrote: > On 2018-11-26 15:09:43 -0300, Alvaro Herrera wrote: > > On 2018-Nov-26, Stephen Frost wrote: > > > > > I would think we'd have the different GUCs and then the check functions > > > would only validate that they're valid inputs and

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Andres Freund
On 2018-11-26 15:09:43 -0300, Alvaro Herrera wrote: > On 2018-Nov-26, Stephen Frost wrote: > > > I would think we'd have the different GUCs and then the check functions > > would only validate that they're valid inputs and then when we get to > > the point where we're starting to do recovery we

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Alvaro Herrera
On 2018-Nov-26, Stephen Frost wrote: > I would think we'd have the different GUCs and then the check functions > would only validate that they're valid inputs and then when we get to > the point where we're starting to do recovery we check and make sure > we've been given a sane overall

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, * Sergei Kornilov (s...@zsrv.org) wrote: > > I would think we'd have the different GUCs and then the check functions > > would only validate that they're valid inputs and then when we get to > > the point where we're starting to do recovery we check and make sure > > we've been given a

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hello > I would think we'd have the different GUCs and then the check functions > would only validate that they're valid inputs and then when we get to > the point where we're starting to do recovery we check and make sure > we've been given a sane overall configuration- which means that only >

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread David Steele
On 11/26/18 10:35 AM, Abhijit Menon-Sen wrote: At 2018-11-26 10:18:52 -0500, sfr...@snowman.net wrote: This came originally to check that recovery.conf handles correctly its recovery targets when multiple ones are defined with the last one winning […] Ugh, no, I don't agree that this

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Abhijit Menon-Sen
At 2018-11-26 10:18:52 -0500, sfr...@snowman.net wrote: > > > This came originally to check that recovery.conf handles correctly > > its recovery targets when multiple ones are defined with the last > > one winning […] > > Ugh, no, I don't agree that this property makes sense to keep and > since

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Stephen Frost
Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Mon, Nov 26, 2018 at 02:06:36PM +0100, Peter Eisentraut wrote: > > What is the reason for allowing multiple recovery_target_* settings and > > taking the last one? > > This came originally to check that recovery.conf handles

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hi > What is the reason for allowing multiple recovery_target_* settings and > taking the last one? Could we change this aspect to make this behave > better? Correct response to user configuration, similar to old recovery.conf logic or other GUC - we allow redefine with rule "latest win". I

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Michael Paquier
On Mon, Nov 26, 2018 at 02:06:36PM +0100, Peter Eisentraut wrote: > What is the reason for allowing multiple recovery_target_* settings and > taking the last one? This came originally to check that recovery.conf handles correctly its recovery targets when multiple ones are defined with the last

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Peter Eisentraut
On 26/11/2018 13:32, Sergei Kornilov wrote: >> Timed out while waiting for standby to catch up at >> t/003_recovery_targets.pl line 34. > > I can reproduce this and notice wrong assign settings order. For example > standby_6 has >> recovery_target_name = '$recovery_name' >>

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-26 Thread Sergei Kornilov
Hi > The buildfarm is unhappy after this one: > https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=culicidae=HEAD > > If I use -DEXEC_BACKEND when compiling the tests complain about a > timeout in 003_recovery_targets. Here is what the buildfarm reports, I > can see the failure by myself

Re: pgsql: Integrate recovery.conf into postgresql.conf

2018-11-25 Thread Michael Paquier
On Sun, Nov 25, 2018 at 03:49:23PM +, Peter Eisentraut wrote: > Integrate recovery.conf into postgresql.conf > > recovery.conf settings are now set in postgresql.conf (or other GUC > sources). Currently, all the affected settings are PGC_POSTMASTER; > this could be refined in the future case