Allow some recovery parameters to be changed with reload

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Allow some recovery parameters to be changed with reload

Peter Eisentraut-6
I think the recovery parameters

    archive_cleanup_command
    promote_trigger_file
    recovery_end_command
    recovery_min_apply_delay

can be changed from PGC_POSTMASTER to PGC_SIGHUP without any further
complications (unlike for example primary_conninfo, which is being
discussed elsewhere).

See attached patch.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

0001-Allow-some-recovery-parameters-to-be-changed-with-re.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Michael Paquier-2
On Mon, Feb 04, 2019 at 11:58:28AM +0100, Peter Eisentraut wrote:
> I think the recovery parameters
>
>     archive_cleanup_command

Only triggered by the checkpointer.

>     promote_trigger_file
>     recovery_end_command
>     recovery_min_apply_delay

Only looked at by the startup process.

> can be changed from PGC_POSTMASTER to PGC_SIGHUP without any further
> complications (unlike for example primary_conninfo, which is being
> discussed elsewhere).

I agree that this subset is straight-forward and safe to switch.  The
documentation changes look right.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Peter Eisentraut-6
On 05/02/2019 04:35, Michael Paquier wrote:

> On Mon, Feb 04, 2019 at 11:58:28AM +0100, Peter Eisentraut wrote:
>> I think the recovery parameters
>>
>>     archive_cleanup_command
>
> Only triggered by the checkpointer.
>
>>     promote_trigger_file
>>     recovery_end_command
>>     recovery_min_apply_delay
>
> Only looked at by the startup process.
>
>> can be changed from PGC_POSTMASTER to PGC_SIGHUP without any further
>> complications (unlike for example primary_conninfo, which is being
>> discussed elsewhere).
>
> I agree that this subset is straight-forward and safe to switch.  The
> documentation changes look right.

Committed, thanks.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Sergei Kornilov
In reply to this post by Michael Paquier-2
Hello

>>  I think the recovery parameters
>>
>>      archive_cleanup_command
>
> Only triggered by the checkpointer.
>
>>      promote_trigger_file
>>      recovery_end_command
>>      recovery_min_apply_delay
>
> Only looked at by the startup process.

We have some possible trouble with restore_command? As far i know it also only looked at by the startup process.

regards, Sergei

Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Peter Eisentraut-6
On 07/02/2019 09:14, Sergei Kornilov wrote:
> We have some possible trouble with restore_command? As far i know it also only looked at by the startup process.

Probably right.  I figured it would be useful to see what the outcome is
with primary_conninfo, so they can be treated similarly.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Michael Paquier-2
On Thu, Feb 07, 2019 at 11:06:27PM +0100, Peter Eisentraut wrote:
> Probably right.  I figured it would be useful to see what the outcome is
> with primary_conninfo, so they can be treated similarly.

The interactions with waiting for WAL to be available and the WAL
receiver stresses me a bit for restore_command, as you could finish
with the startup process switching to use restore_command with a WAL
receiver still working behind and overwriting partially the recovered
segment, which could lead to corruption.  We should be *very* careful
about that.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Andres Freund
Hi,

On 2019-02-08 09:19:31 +0900, Michael Paquier wrote:

> On Thu, Feb 07, 2019 at 11:06:27PM +0100, Peter Eisentraut wrote:
> > Probably right.  I figured it would be useful to see what the outcome is
> > with primary_conninfo, so they can be treated similarly.
>
> The interactions with waiting for WAL to be available and the WAL
> receiver stresses me a bit for restore_command, as you could finish
> with the startup process switching to use restore_command with a WAL
> receiver still working behind and overwriting partially the recovered
> segment, which could lead to corruption.  We should be *very* careful
> about that.

I'm not clear on the precise mechanics you're imagining here, could you
expand a bit? We kill the walreceiver when switching from receiver to
restore command, and wait for it to acknowledge that, no?
C.F. ShutdownWalRcv() call in the lastSourceFailed branch of
WaitForWALToBecomeAvailable().

Greetings,

Andres Freund

Reply | Threaded
Open this post in threaded view
|

Re: Allow some recovery parameters to be changed with reload

Sergei Kornilov
Hello

I want to return to this discussion, since primary_conninfo is now PGC_SIGHUP (and I hope will not be reverted)

> On 2019-02-08 09:19:31 +0900, Michael Paquier wrote:
>>  On Thu, Feb 07, 2019 at 11:06:27PM +0100, Peter Eisentraut wrote:
>>  > Probably right. I figured it would be useful to see what the outcome is
>>  > with primary_conninfo, so they can be treated similarly.
>>
>>  The interactions with waiting for WAL to be available and the WAL
>>  receiver stresses me a bit for restore_command, as you could finish
>>  with the startup process switching to use restore_command with a WAL
>>  receiver still working behind and overwriting partially the recovered
>>  segment, which could lead to corruption. We should be *very* careful
>>  about that.
>
> I'm not clear on the precise mechanics you're imagining here, could you
> expand a bit? We kill the walreceiver when switching from receiver to
> restore command, and wait for it to acknowledge that, no?
> C.F. ShutdownWalRcv() call in the lastSourceFailed branch of
> WaitForWALToBecomeAvailable().
So...
We call restore_command only when walreceiver is stopped.
We use restore_command only in startup process - so we have no race condition between processes.
We have some issues here? Or we can just make restore_command reloadable as attached?

regards, Sergei

v1_restore_command_reload.patch (2K) Download Attachment