Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.
Date: 2016-02-17 15:04:54
Message-ID: 16182.1455721494@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Feb 16, 2016 at 2:59 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Not to be neglected also is that (I believe) this gives the right answer,
>> whereas isolationtester's existing query is currently completely broken by
>> parallel queries, and it doesn't understand non-conflicting lock modes
>> either. (It did, at least partially, before commit 38f8bdcac4982215;
>> I am not sure that taking out the mode checks was a good idea. But
>> putting them back would make the query slower yet.)

> The reason I took that out is because it breaks the deadlock-soft
> test. It's possible to have a situation where no granted lock
> conflicts with an awaited lock. If that happens, the old query
> wrongly concluded that the waiting process was not in fact waiting.
> (Consider A hold AccessShareLock, B awaits AccessExclusiveLock, C now
> requests AccessShareLock and *waits*.)

Ah, well, with this patch the deadlock-soft test still passes.

> As for the patch itself, I'm having trouble grokking what it's trying
> to do. I think it might be worth having a comment defining precisely
> what we mean by "A blocks B". I would define "A blocks B" in general
> as either A holds a lock which conflicts with one sought by B
> (hard-blocked) or A awaits a lock which conflicts with one sought by B
> and precedes it in the wait queue (soft-blocked).

Yes, that is exactly what I implemented ... and it's something you can't
find out from pg_locks. I'm not sure how that view could be made to
expose wait-queue ordering.

> For parallel queries, there's a further relevant distinction when we
> say "A blocks B". We might mean either that (1) process B cannot
> resume execution until the lock conflict is resolved or (2) the group
> leader for process B cannot complete the current parallel operation
> until the lock conflict is resolved.

The definition I used in this patch is "some member of A's lock group
blocks some member of B's lock group", because that corresponds directly
to whether A is preventing B's query from completing, which is what
isolationtester wants to know --- and, I would argue, it's generally what
any client would want to know. 99.9% of clients would just as soon not be
aware of parallel workers lurking underneath the pg_backend_pid() values
that they see for their sessions.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2016-02-17 16:18:11 Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.
Previous Message Robert Haas 2016-02-17 14:52:12 Re: Re: [COMMITTERS] pgsql: Add some isolation tests for deadlock detection and resolution.

Browse pgsql-hackers by date

  From Date Subject
Next Message Catalin Iacob 2016-02-17 15:54:29 Re: proposal: PL/Pythonu - function ereport
Previous Message Dean Rasheed 2016-02-17 15:03:32 Re: [patch] Proposal for \crosstabview in psql