Re: Deadlock between backend and recovery may not be detected

On 2020/12/25 13:16, Kyotaro Horiguchi wrote:
> At Wed, 23 Dec 2020 21:42:47 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>> you. Attached
>> is the updated of the patch. What about this version?
>
> The patch contains a hunk in the following structure.
>
> + if (got_standby_lock_timeout)
> + goto cleanup;
> +
> + if (got_standby_deadlock_timeout)
> + {
> ...
> + }
> +
> +cleanup:
>
> It is eqivalent to
>
> + if (!got_standby_lock_timeout && got_standby_deadlock_timeout)
> + {
> ...
> + }
>
> Is there any reason for the goto?

Yes. That's because we have the following code using goto.

+ /* Quick exit if there's no work to be done */
+ if (!VirtualTransactionIdIsValid(*backends))
+ goto cleanup;

Regarding the back-patch, I was thinking to back-patch this to all the
supported branches. But I found that it's not easy to do that to v9.5
because v9.5 doesn't have some infrastructure code that this bug fix
patch depends on. So at least the commit 37c54863cf as the infrastructure
also needs to be back-patched to v9.5. And ISTM that some related commits
f868a8143a and 8f0de712c3 need to be back-patched. Probably there might
be some other changes to be back-patched. Unfortunately they cannot be
applied to v9.5 cleanly and additional changes are necessary.

This situation makes me feel that I'm inclined to skip the back-patch to v9.5.
Because the next minor version release is the final one for v9.5. So if we
unexpectedly introduce the bug to v9.5 by the back-patch, there is no
chance to fix that. OTOH, of course, if we don't do the back-patch, there is
no chance to fix the deadlock detection bug since the final minor version
for v9.5 doesn't include that bug fix. But I'm afraid that the back-patch
to v9.5 may give more risk than gain.

Thought?

Regards,

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