Re: Fast promotion, loose ends

On 24.04.2013 10:57, Simon Riggs wrote:
> On 24 April 2013 08:23, Heikki Linnakangas<hlinnakangas@vmware.com>  wrote:
>> On 22.04.2013 18:44, Simon Riggs wrote:
>>>
>>> On 22 April 2013 09:29, Heikki Linnakangas<hlinnakangas@vmware.com>
>>> wrote:
>>>
>>>> Hmm. That requires write access to $DATADIR, so that's not quite the same
>>>> thing as the trigger_file recovery.conf option.
>>>
>>> Well, you also (elsewhere) requested that we must keep recovery.conf
>>> in $DATADIR, so it needs to be writable.
>>
>> That's a slightly different requirement. $DATADIR must be writable by the
>> process that restores the backup or puts the server into standby mode, while
>> trigger_file needs to be writable by the process that triggers failover.
>> Those are not necessarily the same thing. I'm thinking of a heartbeat
>> process that triggers failover by creating a file on an NFS server or
>> similar. Possibly the same location where the WAL archive is located.
>> $DATADIR would be stored elsewhere.
>
> The default you've requested is fast promotion and I've agreed to that.
>
> The ability to write a file called "promote" to $DATADIR is there as a
> protection in case we need it in the field, its not going to be the
> primary mechanism any more. So if you're not intending to use it ever,
> it doesn't seem worth discussing the fact you don't like its location.

Ok, works for me.

> But if you do want to discuss it, I think it's unreasonable of you to
> demand recovery.conf cannot be outside $DATADIR and then also demand
> related files be relocatable outside $DATADIR as well, the exact
> opposite. We had the chance to avoid making $DATADIR writable
> externally and that's gone now, at least for now.

As I said above, it's a different situation. recovery.conf has always 
been in $DATADIR, and it's always been possible to point trigger_file 
elsewhere, and you've always gotten full functionality using the 
trigger_file. I just want to maintain that status quo. Which your patch 
achieves, so I'm happy that that.

> Here's the patch I was intending to apply. Please let me know if you
> have comments.

Regarding the change in pg_ctl:

>      /*
> -     * Use two different kinds of promotion file so we can understand
> -     * the difference between smart and fast promotion.
> +     * For 9.3 onwards, use fast promotion as the default option.
> +     * Promotion with a full checkpoint is still possible by writing
> +     * a file called "promote", e.g.
> +     *      snprintf(promote_file, MAXPGPATH, "%s/promote", pg_data);
>       */
> -    if (shutdown_mode >= FAST_MODE)
> -        snprintf(promote_file, MAXPGPATH, "%s/fast_promote", pg_data);
> -    else
> -        snprintf(promote_file, MAXPGPATH, "%s/promote", pg_data);
> +    snprintf(promote_file, MAXPGPATH, "%s/fast_promote", pg_data);

Should there be a version check there? I guess we've never guaranteed a 
newer pg_ctl to work with an older server version, but it seems likely 
that someone would try to do that, especially with "pg_ctl promote". 
With the above change, creating $DATADIR/fast_promote in a 9.2 server's 
data dir will do nothing. I'd suggest that we keep the filename 
unchanged, "promote", and only change the behavior in the server side, 
so that it performs fast promotion. If you want to have a "slow" promote 
file, we can call that "slow_promote" or "checkpoint_then_promote" or 
something.

Aside from that, I assume you'll clean up the now-dead -m fast option etc.

- Heikki