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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
