Thread: comment for "fast promote"

comment for "fast promote"

From
Tomonari Katsumata
Date:
Hi,

Now I'm seeing xlog.c in 93_stable for studying "fast promote",
and I have a question.

I think it has an extra unlink command for "promote" file.
(on 9937 line)
-------
9934 if (stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
9935 {
9936 unlink(FAST_PROMOTE_SIGNAL_FILE);
9937 unlink(PROMOTE_SIGNAL_FILE);
9938 fast_promote = true;
9939 }
-------

Is this command necesary ?

regards,
------------------
NTT Software Corporation
Tomonari Katsumata






Re: comment for "fast promote"

From
Fujii Masao
Date:
On Thu, Jul 25, 2013 at 5:33 PM, Tomonari Katsumata
<katsumata.tomonari@po.ntts.co.jp> wrote:
> Hi,
>
> Now I'm seeing xlog.c in 93_stable for studying "fast promote",
> and I have a question.
>
> I think it has an extra unlink command for "promote" file.
> (on 9937 line)
> -------
> 9934 if (stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
> 9935 {
> 9936 unlink(FAST_PROMOTE_SIGNAL_FILE);
> 9937 unlink(PROMOTE_SIGNAL_FILE);
> 9938 fast_promote = true;
> 9939 }
> -------
>
> Is this command necesary ?

Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
both promote files exist.

One question is that: we really still need to support normal promote?
pg_ctl promote provides only way to do fast promotion. If we want to
do normal promotion, we need to create PROMOTE_SIGNAL_FILE
and send the SIGUSR1 signal to postmaster by hand. This seems messy.

I think that we should remove normal promotion at all, or change
pg_ctl promote so that provides also the way to do normal promotion.

Regards,

-- 
Fujii Masao



Re: comment for "fast promote"

From
Tomonari Katsumata
Date:
Hi Fujii-san,

Thank you for response.

(2013/07/25 21:15), Fujii Masao wrote:> On Thu, Jul 25, 2013 at 5:33 PM, Tomonari Katsumata>
<katsumata.tomonari@po.ntts.co.jp>wrote:>> Hi,>>>> Now I'm seeing xlog.c in 93_stable for studying "fast promote",>>
andI have a question.>>>> I think it has an extra unlink command for "promote" file.>> (on 9937 line)>> ------->> 9934
if(stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)>> 9935 {>> 9936 unlink(FAST_PROMOTE_SIGNAL_FILE);>> 9937
unlink(PROMOTE_SIGNAL_FILE);>>9938 fast_promote = true;>> 9939 }>> ------->>>> Is this command necesary ?>> Yes, it
preventsPROMOTE_SIGNAL_FILE from remaining even if> both promote files exist.>
 
The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
unusualy case.
Because the case is when done both procedures below. - user create "promote" file on PGDATA - user issue "pg_ctl
promote"

I understand the reason.
But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
unlink(FAST_PROMOTE_SIGNAL_FILE).
Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
PROMOTE_SIGNAL_FILE is sometimes there or not there.

And I have another question linking this behavior.
I think TriggerFile should be removed too.
This is corner-case but it will happen.
How do you think of it ?
> One question is that: we really still need to support normal promote?> pg_ctl promote provides only way to do fast
promotion.If we want to> do normal promotion, we need to create PROMOTE_SIGNAL_FILE> and send the SIGUSR1 signal to
postmasterby hand. This seems messy.>> I think that we should remove normal promotion at all, or change> pg_ctl promote
sothat provides also the way to do normal promotion.>
 
I think he merit of "fast promote" is - allowing quick connection by skipping checkpoint
and its demerit is - taking little bit longer when crash-recovery

If it is seldom to happen its crash soon after promoting
and "fast promte" never breaks consistency of database cluster,
I think we don't need normal promotion.
(of course we need to put detail about promotion on document.)

regards,
--------
NTT Software Corporation
Tomonari Katsumata






Re: comment for "fast promote"

From
Fujii Masao
Date:
On Fri, Jul 26, 2013 at 11:19 AM, Tomonari Katsumata
<katsumata.tomonari@po.ntts.co.jp> wrote:
> Hi Fujii-san,
>
> Thank you for response.
>
>
> (2013/07/25 21:15), Fujii Masao wrote:
>> On Thu, Jul 25, 2013 at 5:33 PM, Tomonari Katsumata
>> <katsumata.tomonari@po.ntts.co.jp> wrote:
>>> Hi,
>>>
>>> Now I'm seeing xlog.c in 93_stable for studying "fast promote",
>>> and I have a question.
>>>
>>> I think it has an extra unlink command for "promote" file.
>>> (on 9937 line)
>>> -------
>>> 9934 if (stat(FAST_PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
>>> 9935 {
>>> 9936 unlink(FAST_PROMOTE_SIGNAL_FILE);
>>> 9937 unlink(PROMOTE_SIGNAL_FILE);
>>> 9938 fast_promote = true;
>>> 9939 }
>>> -------
>>>
>>> Is this command necesary ?
>>
>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
>> both promote files exist.
>>
> The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
> unusualy case.
> Because the case is when done both procedures below.
>  - user create "promote" file on PGDATA
>  - user issue "pg_ctl promote"
>
> I understand the reason.
> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
> unlink(FAST_PROMOTE_SIGNAL_FILE).
> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
> PROMOTE_SIGNAL_FILE is sometimes there or not there.

I could not understand why that's better. Could you elaborate that?

> And I have another question linking this behavior.
> I think TriggerFile should be removed too.
> This is corner-case but it will happen.
> How do you think of it ?

I don't have strong opinion about that. I've never heard the complaint
about that current behavior so far.

>> One question is that: we really still need to support normal promote?
>> pg_ctl promote provides only way to do fast promotion. If we want to
>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
>> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
>>
>> I think that we should remove normal promotion at all, or change
>> pg_ctl promote so that provides also the way to do normal promotion.
>>
> I think he merit of "fast promote" is
>  - allowing quick connection by skipping checkpoint
> and its demerit is
>  - taking little bit longer when crash-recovery
>
> If it is seldom to happen its crash soon after promoting
> and "fast promte" never breaks consistency of database cluster,
> I think we don't need normal promotion.

You can execute checkpoint after fast promotion for that.

Regards,

-- 
Fujii Masao



Re: comment for "fast promote"

From
Tomonari Katsumata
Date:
<div dir="ltr">Hi,<br /><br />>>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if<br
/>>>>both promote files exist.<br />>>><br />>> The command("unlink(PROMOTE_SIGNAL_FILE)") here
isfor<br /> >> unusualy case.<br />>> Because the case is when done both procedures below.<br />>>  -
usercreate "promote" file on PGDATA<br />>>  - user issue "pg_ctl promote"<br />>><br />>> I
understandthe reason.<br /> >> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before<br />>>
unlink(FAST_PROMOTE_SIGNAL_FILE).<br/>>> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but<br />>>
PROMOTE_SIGNAL_FILEis sometimes there or not there.<br /> ><br />> I could not understand why that's better.
Couldyou elaborate that?<br />><br />I'm sorry for less explanation.<br /><br />I've thought that errno would be set
ENOENTand<br />this may lead something wrong.<br /> I checked this and I know it's not problem.<br /><br />sorry for
confusingyou.<br /><br /><br />>> And I have another question linking this behavior.<br />>> I think
TriggerFileshould be removed too.<br />>> This is corner-case but it will happen.<br /> >> How do you think
ofit ?<br />><br />> I don't have strong opinion about that. I've never heard the complaint<br />> about that
currentbehavior so far.<br />><br />For example, please imagine the cascading replication environment and<br />
usingold master as a standby without copying the timeline history file<br />to new standby.<br /><br />-------<br />1.
replicating3 servers(A,B,C)<br />A->B->C<br />("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B
andC.)<br /><br />2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl promote"<br />B->C<br
/>(/tmp/trigfile remains on server B)<br /><br />4. stop server B and promoting server C with "pg_ctl promote"<br />
C<br/><br />5. making server B connect for standby of server C<br />C->B<br />---------<br /><br />In step5 server B
willpromote as soon as it starts,<br />because "/tmp/trig" is stil there.<br /><br /><br />>>> One question is
that:we really still need to support normal promote?<br /> >>> pg_ctl promote provides only way to do fast
promotion.If we want to<br />>>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE<br />>>>
andsend the SIGUSR1 signal to postmaster by hand. This seems messy.<br /> >>><br />>>> I think that
weshould remove normal promotion at all, or change<br />>>> pg_ctl promote so that provides also the way to do
normalpromotion.<br />>>><br />>> I think he merit of "fast promote" is<br /> >>  - allowing quick
connectionby skipping checkpoint<br />>> and its demerit is<br />>>  - taking little bit longer when
crash-recovery<br/>>><br />>> If it is seldom to happen its crash soon after promoting<br /> >> and
"fastpromte" never breaks consistency of database cluster,<br />>> I think we don't need normal promotion.<br
/>><br/>> You can execute checkpoint after fast promotion for that.<br />><br /> OK.<br />Then I think we
shoulddo below things.<br />- removing normal promotion at all from source<br />- adding the know-how you suggest on
document<br/><br />Are there any objection?<br /><br />regards,<br />-------------------<br />Tomonari Katsumata<br
/><br/></div> 

Re: comment for "fast promote"

From
Euler Taveira
Date:
On 27-07-2013 06:57, Tomonari Katsumata wrote:
> 1. replicating 3 servers(A,B,C)
> A->B->C
> ("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B and C.)
> 
> 2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl
> promote"
> B->C
> (/tmp/trig file remains on server B)
> 
Why don't you setup recovery_end_command parameter? The trigger_file is
important in some (legacy) environments and that is using an external
tool to handle the service initialization.

It seems to me it is an opportunity to improve trigger_file description
(informing a way to cleanup the file created) than to suggest it is not
useful.


--   Euler Taveira                   Timbira - http://www.timbira.com.br/  PostgreSQL: Consultoria, Desenvolvimento,
Suporte24x7 e Treinamento
 



Re: comment for "fast promote"

From
Fujii Masao
Date:
On Sat, Jul 27, 2013 at 6:57 PM, Tomonari Katsumata
<t.katsumata1122@gmail.com> wrote:
> Hi,
>
>
>>>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
>>>> both promote files exist.
>>>>
>>> The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
>>> unusualy case.
>>> Because the case is when done both procedures below.
>>>  - user create "promote" file on PGDATA
>>>  - user issue "pg_ctl promote"
>>>
>>> I understand the reason.
>>> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
>>> unlink(FAST_PROMOTE_SIGNAL_FILE).
>>> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
>>> PROMOTE_SIGNAL_FILE is sometimes there or not there.
>>
>> I could not understand why that's better. Could you elaborate that?
>>
> I'm sorry for less explanation.
>
> I've thought that errno would be set ENOENT and
> this may lead something wrong.
> I checked this and I know it's not problem.
>
> sorry for confusing you.
>
>
>
>>> And I have another question linking this behavior.
>>> I think TriggerFile should be removed too.
>>> This is corner-case but it will happen.
>>> How do you think of it ?
>>
>> I don't have strong opinion about that. I've never heard the complaint
>> about that current behavior so far.
>>
> For example, please imagine the cascading replication environment and
> using old master as a standby without copying the timeline history file
> to new standby.
>
> -------
> 1. replicating 3 servers(A,B,C)
> A->B->C
> ("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B and C.)
>
> 2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl
> promote"

Why do you need to both create the trigger file and run pg_ctl promote?

Anyway, if the patch is useful for fail-safe and it doesn't break the current
behavior, I'd be happy to apply it. You are suggesting that we should remove
the trigger file in CheckForStandbyTrigger() even if pg_ctl promote is executed.
But there can be some cases where we can get out of the WAL replay loop,
for example, reach the recovery_target_xxx. So ISTM we should try to remove
both the trigger file and "promote" file at the end of recovery
instead. Thought?

> B->C
> (/tmp/trig file remains on server B)
>
> 4. stop server B and promoting server C with "pg_ctl promote"
> C
>
> 5. making server B connect for standby of server C
> C->B
> ---------
>
> In step5 server B will promote as soon as it starts,
> because "/tmp/trig" is stil there.
>
>
>
>>>> One question is that: we really still need to support normal promote?
>>>> pg_ctl promote provides only way to do fast promotion. If we want to
>>>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
>>>> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
>>>>
>>>> I think that we should remove normal promotion at all, or change
>>>> pg_ctl promote so that provides also the way to do normal promotion.
>>>>
>>> I think he merit of "fast promote" is
>>>  - allowing quick connection by skipping checkpoint
>>> and its demerit is
>>>  - taking little bit longer when crash-recovery
>>>
>>> If it is seldom to happen its crash soon after promoting
>>> and "fast promte" never breaks consistency of database cluster,
>>> I think we don't need normal promotion.
>>
>> You can execute checkpoint after fast promotion for that.
>>
> OK.
> Then I think we should do below things.
> - removing normal promotion at all from source
> - adding the know-how you suggest on document

IMO either is necessary.

Regards,

-- 
Fujii Masao



Re: comment for "fast promote"

From
Tomonari Katsumata
Date:
Hi,
 
I made a patch for REL9_3_STABLE which gets rid of
old promote processing. please check it.
This patch make PostgreSQL do fast promoting(*) always.
(*) which means skipping long checkpoint before increasing
timeline.
 
And after this, I'll do make another patch for unlinking files which are
created by user as a trigger_file or "pg_ctl promote" command.

---------------
Tomonari Katsumata
2013/7/30 Fujii Masao <masao.fujii@gmail.com>
On Sat, Jul 27, 2013 at 6:57 PM, Tomonari Katsumata
<t.katsumata1122@gmail.com> wrote:
> Hi,
>
>
>>>> Yes, it prevents PROMOTE_SIGNAL_FILE from remaining even if
>>>> both promote files exist.
>>>>
>>> The command("unlink(PROMOTE_SIGNAL_FILE)") here is for
>>> unusualy case.
>>> Because the case is when done both procedures below.
>>>  - user create "promote" file on PGDATA
>>>  - user issue "pg_ctl promote"
>>>
>>> I understand the reason.
>>> But I think it's better to unlink(PROMOTE_SIGNAL_FILE) before
>>> unlink(FAST_PROMOTE_SIGNAL_FILE).
>>> Because FAST_PROMOTE_SIGNAL_FILE is definetly there but
>>> PROMOTE_SIGNAL_FILE is sometimes there or not there.
>>
>> I could not understand why that's better. Could you elaborate that?
>>
> I'm sorry for less explanation.
>
> I've thought that errno would be set ENOENT and
> this may lead something wrong.
> I checked this and I know it's not problem.
>
> sorry for confusing you.
>
>
>
>>> And I have another question linking this behavior.
>>> I think TriggerFile should be removed too.
>>> This is corner-case but it will happen.
>>> How do you think of it ?
>>
>> I don't have strong opinion about that. I've never heard the complaint
>> about that current behavior so far.
>>
> For example, please imagine the cascading replication environment and
> using old master as a standby without copying the timeline history file
> to new standby.
>
> -------
> 1. replicating 3 servers(A,B,C)
> A->B->C
> ("trigger_file = /tmp/trig" is set in recovery_recovery.conf on B and C.)
>
> 2. stop server A and promoting server B with "touch /tmp/trig;pg_ctl
> promote"

Why do you need to both create the trigger file and run pg_ctl promote?

Anyway, if the patch is useful for fail-safe and it doesn't break the current
behavior, I'd be happy to apply it. You are suggesting that we should remove
the trigger file in CheckForStandbyTrigger() even if pg_ctl promote is executed.
But there can be some cases where we can get out of the WAL replay loop,
for example, reach the recovery_target_xxx. So ISTM we should try to remove
both the trigger file and "promote" file at the end of recovery
instead. Thought?

> B->C
> (/tmp/trig file remains on server B)
>
> 4. stop server B and promoting server C with "pg_ctl promote"
> C
>
> 5. making server B connect for standby of server C
> C->B
> ---------
>
> In step5 server B will promote as soon as it starts,
> because "/tmp/trig" is stil there.
>
>
>
>>>> One question is that: we really still need to support normal promote?
>>>> pg_ctl promote provides only way to do fast promotion. If we want to
>>>> do normal promotion, we need to create PROMOTE_SIGNAL_FILE
>>>> and send the SIGUSR1 signal to postmaster by hand. This seems messy.
>>>>
>>>> I think that we should remove normal promotion at all, or change
>>>> pg_ctl promote so that provides also the way to do normal promotion.
>>>>
>>> I think he merit of "fast promote" is
>>>  - allowing quick connection by skipping checkpoint
>>> and its demerit is
>>>  - taking little bit longer when crash-recovery
>>>
>>> If it is seldom to happen its crash soon after promoting
>>> and "fast promte" never breaks consistency of database cluster,
>>> I think we don't need normal promotion.
>>
>> You can execute checkpoint after fast promotion for that.
>>
> OK.
> Then I think we should do below things.
> - removing normal promotion at all from source
> - adding the know-how you suggest on document

IMO either is necessary.

Regards,

--
Fujii Masao

Attachment

Re: comment for "fast promote"

From
Fujii Masao
Date:
On Sat, Aug 3, 2013 at 4:31 PM, Tomonari Katsumata
<t.katsumata1122@gmail.com> wrote:
> Hi,
>
> I made a patch for REL9_3_STABLE which gets rid of
> old promote processing. please check it.
> This patch make PostgreSQL do fast promoting(*) always.
> (*) which means skipping long checkpoint before increasing
> timeline.

Thanks for the patch!

I fixed the bug that your patch accidentally makes archive recovery
skip end-of-recovery checkpoint, fixed some typos, refactored the
source code and posted the updated version of the patch in
http://www.postgresql.org/message-id/CAHGQGwGYkF+CvpOMdxaO=+aNAzc1Oo9O4LqWo50MxpvFj+0VOw@mail.gmail.com

Regards,

-- 
Fujii Masao