Re: Add shutdown_at_recovery_target option to recovery.conf - Mailing list pgsql-hackers
From | Asif Naeem |
---|---|
Subject | Re: Add shutdown_at_recovery_target option to recovery.conf |
Date | |
Msg-id | CAEB4t-N5yq1dmKNkYVL2B+etKKjqBUYv0B_1YBXgkYvNm5zrzg@mail.gmail.com Whole thread Raw |
In response to | Re: Add shutdown_at_recovery_target option to recovery.conf (Simon Riggs <simon@2ndQuadrant.com>) |
Responses |
Re: Add shutdown_at_recovery_target option to recovery.conf
|
List | pgsql-hackers |
Hi Petr,
I have spent sometime to review the patch, overall patch looks good, it applies fine and make check run without issue. If recovery target is specified and shutdown_at_recovery_target is set to true, it shutdown the server at specified recovery point. I do have few points to share i.e.
1. It seems that following log message need to be more descriptive about reason for shutdown i.e.
2. As Simon suggesting following recovery settings are not clear i.e.
It is going to make true both but patch describe as following i.e.
3. As it don’t rename reconvery.conf, subsequent attempt (without any changes in reconvery.conf) to start of server keep showing the following i.e.
Is that right ?. Thanks.
Regards,
Muhammad Asif Naeem
I have spent sometime to review the patch, overall patch looks good, it applies fine and make check run without issue. If recovery target is specified and shutdown_at_recovery_target is set to true, it shutdown the server at specified recovery point. I do have few points to share i.e.
1. It seems that following log message need to be more descriptive about reason for shutdown i.e.
+ if (recoveryShutdownAtTarget && reachedStopPoint)
+ {
+ ereport(LOG, (errmsg("shutting down")));
2. As Simon suggesting following recovery settings are not clear i.e.
shutdown_at_recovery_target = true
pause_at_recovery_target = true
It is going to make true both but patch describe as following i.e.
+ Setting this to true will set <link linkend="pause-at-recovery-target">
+ <varname>pause_at_recovery_target</></link> to false.
...
LOG: redo starts at 0/1803620
DEBUG: checkpointer updated shared memory configuration values
LOG: consistent recovery state reached at 0/1803658
LOG: restored log file "000000010000000000000002" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000003" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000004" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000005" from archive
DEBUG: got WAL segment from archive
LOG: restored log file "000000010000000000000006" from archive
DEBUG: got WAL segment from archive
…
Regards,
Muhammad Asif Naeem
On Thu, Oct 16, 2014 at 7:24 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 11 September 2014 16:02, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> What about adding something like action_at_recovery_target=pause|shutdown
>> instead of increasing the number of parameters?
>>
>
> That will also increase number of parameters as we can't remove the current
> pause one if we want to be backwards compatible. Also there would have to be
> something like action_at_recovery_target=none or off or something since the
> default is that pause is on and we need to be able to turn off pause without
> having to have shutdown on. What more, I am not sure I see any other actions
> that could be added in the future as promote action already works and listen
> (for RO queries) also already works independently of this.
I accept your argument, though I have other thoughts.
If someone specifies
shutdown_at_recovery_target = true
pause_at_recovery_target = true
it gets a little hard to work out what to do; we shouldn't allow such
lack of clarity.
In recovery its easy to do this
if (recoveryShutdownAtTarget)
recoveryPauseAtTarget = false;
but it won't be when these become GUCs, so I think Fuji's suggestion
is a good one.
No other comments on patch, other than good idea.
--
Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
pgsql-hackers by date: