On 29/10/14 20:27, Asif Naeem wrote:
> 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.
>
Thanks!
> 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.
> 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.
>
> 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.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
