Thread: STANDBY_LOCK_TIMEOUT may not interrupt ProcWaitForSignal()?

Hi,

When the startup process needs to wait for recovery conflict on lock,
STANDBY_LOCK_TIMEOUT is enabled to interrupt ProcWaitForSignal()
if necessary. If this timeout happens, StandbyLockTimeoutHandler() is
called and this function does nothing as follows.

     /*
      * StandbyLockTimeoutHandler() will be called if STANDBY_LOCK_TIMEOUT is exceeded.
      * This doesn't need to do anything, simply waking up is enough.
      */
     void
     StandbyLockTimeoutHandler(void)
     {
     }

But if STANDBY_LOCK_TIMEOUT happens just before entering ProcWaitForSignal(),
the timeout fails to interrupt that wait. Also a signal sent by this timeout
doesn't interrupt poll() used in ProcWaitForSignal(), on all platforms.

So I think that StandbyLockTimeoutHandler() should do SetLatch(MyLatch)
so that the timeout can interrupt ProcWaitForSignal() even in those cases.
Thought?

Regards,

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

On 2020/12/17 11:04, Fujii Masao wrote:
> Hi,
> 
> When the startup process needs to wait for recovery conflict on lock,
> STANDBY_LOCK_TIMEOUT is enabled to interrupt ProcWaitForSignal()
> if necessary. If this timeout happens, StandbyLockTimeoutHandler() is
> called and this function does nothing as follows.
> 
>      /*
>       * StandbyLockTimeoutHandler() will be called if STANDBY_LOCK_TIMEOUT is exceeded.
>       * This doesn't need to do anything, simply waking up is enough.
>       */
>      void
>      StandbyLockTimeoutHandler(void)
>      {
>      }
> 
> But if STANDBY_LOCK_TIMEOUT happens just before entering ProcWaitForSignal(),
> the timeout fails to interrupt that wait. Also a signal sent by this timeout
> doesn't interrupt poll() used in ProcWaitForSignal(), on all platforms.
> 
> So I think that StandbyLockTimeoutHandler() should do SetLatch(MyLatch)
> so that the timeout can interrupt ProcWaitForSignal() even in those cases.
> Thought?

Patch attached.

Regards,

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

On 2020/12/17 18:45, Fujii Masao wrote:
> 
> 
> On 2020/12/17 11:04, Fujii Masao wrote:
>> Hi,
>>
>> When the startup process needs to wait for recovery conflict on lock,
>> STANDBY_LOCK_TIMEOUT is enabled to interrupt ProcWaitForSignal()
>> if necessary. If this timeout happens, StandbyLockTimeoutHandler() is
>> called and this function does nothing as follows.
>>
>>      /*
>>       * StandbyLockTimeoutHandler() will be called if STANDBY_LOCK_TIMEOUT is exceeded.
>>       * This doesn't need to do anything, simply waking up is enough.
>>       */
>>      void
>>      StandbyLockTimeoutHandler(void)
>>      {
>>      }
>>
>> But if STANDBY_LOCK_TIMEOUT happens just before entering ProcWaitForSignal(),
>> the timeout fails to interrupt that wait. Also a signal sent by this timeout
>> doesn't interrupt poll() used in ProcWaitForSignal(), on all platforms.
>>
>> So I think that StandbyLockTimeoutHandler() should do SetLatch(MyLatch)
>> so that the timeout can interrupt ProcWaitForSignal() even in those cases.
>> Thought?

Bertrand Drouvot pointed out that this my analysis is incorrect
because handle_sig_alarm() calls SetLatch(), on twitter. So
StandbyLockTimeoutHandler() doesn't need to call SetLatch().
Yes, he is right. Sorry for my shameful mistake....

I found that other functions, CheckDeadLockAlert() and
IdleInTransactionSessionTimeoutHandler(), that are triggered by
SIGALRM also call SetLatch(). This call to SetLatch() is also unneessary.
Per comment, CheckDeadLockAlert() intentionally does that. But since
setting a latch again is cheap and is not harmful, it would not so worth
removing that.

Regards,

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