Re: Deadlock between backend and recovery may not be detected

Drouvot, Bertrand <bdrouvot@amazon.com>

From: "Drouvot, Bertrand" <bdrouvot@amazon.com>
To: Victor Yegorov <vyegorov@gmail.com>, Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-16T14:28:33Z
Lists: pgsql-hackers
Hi,

On 12/16/20 2:36 PM, Victor Yegorov wrote:
>
> *CAUTION*: This email originated from outside of the organization. Do 
> not click links or open attachments unless you can confirm the sender 
> and know the content is safe.
>
>
> ср, 16 дек. 2020 г. в 13:49, Fujii Masao <masao.fujii@oss.nttdata.com 
> <mailto:masao.fujii@oss.nttdata.com>>:
>
>     After doing this procedure, you can see the startup process and
>     backend
>     wait for the table lock each other, i.e., deadlock. But this
>     deadlock remains
>     even after deadlock_timeout passes.
>
>     This seems a bug to me.
>
+1

>
>     > * Deadlocks involving the Startup process and an ordinary
>     backend process
>     > * will be detected by the deadlock detector within the ordinary
>     backend.
>
>     The cause of this issue seems that
>     ResolveRecoveryConflictWithLock() that
>     the startup process calls when recovery conflict on lock happens
>     doesn't
>     take care of deadlock case at all. You can see this fact by
>     reading the above
>     source code comment for ResolveRecoveryConflictWithLock().
>
>     To fix this issue, I think that we should enable
>     STANDBY_DEADLOCK_TIMEOUT
>     timer in ResolveRecoveryConflictWithLock() so that the startup
>     process can
>     send PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK signal to the backend.
>     Then if PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK signal arrives,
>     the backend should check whether the deadlock actually happens or not.
>     Attached is the POC patch implimenting this.
>
good catch!

I don't see any obvious reasons why the STANDBY_DEADLOCK_TIMEOUT 
shouldn't be set in ResolveRecoveryConflictWithLock() too (it is already 
set in ResolveRecoveryConflictWithBufferPin()).

So + 1 to consider this as a bug and for the way the patch proposes to 
fix it.

Bertrand

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Detect the deadlocks between backends and the startup process.

  2. Rework wait for AccessExclusiveLocks on Hot Standby