Re: Add shutdown_at_recovery_target option to recovery.conf - Mailing list pgsql-hackers

Hi,

Attached is the v2 of the patch with the review comments addressed (see
below).

On 29/10/14 21:08, Petr Jelinek wrote:
> On 29/10/14 20:27, Asif Naeem wrote:
>> 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")));
>>
>
> Agreed, I just wasn't sure on what exactly to writes, I originally had
> there "shutting down by user request" or some such but that's misleading.
>

I changed it to "shutting down at recovery target" hope that's better.

>> 2. As Simon suggesting following recovery settings are not clear i.e.
>>
>>     shutdown_at_recovery_target = true
>>     pause_at_recovery_target = true
>
> Hmm I completely missed Simon's email, strange. Well other option would
> be to throw error if both are set to true - error will have to happen
> anyway if action_at_recovery_target is set to shutdown while
> pause_at_recovery_target is true (I think we need to keep
> pause_at_recovery_target for compatibility).
>
> But considering all of you think something like
> action_at_recovery_target is better solution, I will do it that way then.

Done, there is now action_at_recovery_target which can be set to either
pause, continue or shutdown, defaulting to pause (which is same as old
behavior of pause_at_recovery_target defaulting to true).
I also added check that prohibits using both pause_at_recovery_target
and action_at_recovery_target in the same config, mainly to avoid users
shooting themselves in the foot.

>
>>
>> 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.
>>
>>     ...
>>     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
>>     …
>>
>
> Yes, it will still replay everything since last checkpoint, that's by
> design since otherwise we would have to write checkpoint on shutdown and
> that would mean the instance can't be used as hot standby anymore and I
> consider that an important feature to have.
>

I added note to the documentation that says this will happen.

--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services