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.

+                   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.
 
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

 
Is that right ?. Thanks.

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:

Previous
From: Robert Haas
Date:
Subject: Re: pg_receivexlog --status-interval add fsync feedback
Next
From: Andres Freund
Date:
Subject: Re: pg_background (and more parallelism infrastructure patches)