Re: Remove non-fast promotion Re: Should we remove a fallbackpromotion? take 2

On 2020/04/22 6:57, Alvaro Herrera wrote:
> On 2020-Apr-20, Fujii Masao wrote:
> 
>> +    /*
>> +     * In 9.1 and 9.2 the postmaster unlinked the promote file inside the
>> +     * signal handler. It now leaves the file in place and lets the
>> +     * Startup process do the unlink.
>> +     */
>> +    if (IsPromoteSignaled() && stat(PROMOTE_SIGNAL_FILE, &stat_buf) == 0)
>>       {
>> -        /*
>> -         * In 9.1 and 9.2 the postmaster unlinked the promote file inside the
>> -         * signal handler. It now leaves the file in place and lets the
>> -         * Startup process do the unlink. This allows Startup to know whether
>> -         * it should create a full checkpoint before starting up (fallback
>> -         * mode). Fast promotion takes precedence.
>> -         */
> 
> It seems pointless to leave a very old comment that documents what the
> code no longer does.  I thikn it would be better to reword it indicating
> what the code does do, ie. something like "Leave the signal file in
> place; it will be removed by the startup process when ..."

Agreed. And, while reading the related code, I thought that it's more proper
to place this comment in CheckPromoteSignal() rather than
CheckForStandbyTrigger(). Because CheckPromoteSignal() actually does
what the comment says, i.e., leaves the promote signal file in place and
lets the startup process do the unlink.

Attached is the updated version of the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION