Re: Let's invent a function to report lock-wait-blocking PIDs

On Wed, Mar 20, 2013 at 2:02 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I was just looking into why the -DCLOBBER_CACHE_ALWAYS buildfarm
> critters aren't managing to run the new "timeouts" isolation test
> successfully, despite very generous timeouts. The answer is that
> 2 seconds isn't quite enough time to parse+plan+execute the query
> that isolationtester uses to see if the current test session is
> blocked on a lock, if CLOBBER_CACHE_ALWAYS is on. Now, that query
> is totally horrible:
>
> appendPQExpBufferStr(&wait_query,
> "SELECT 1 FROM pg_locks holder, pg_locks waiter "
> "WHERE NOT waiter.granted AND waiter.pid = $1 "
> "AND holder.granted "
> "AND holder.pid <> $1 AND holder.pid IN (");
> /* The spec syntax requires at least one session; assume that here. */
> appendPQExpBuffer(&wait_query, "%s", backend_pids[1]);
> for (i = 2; i < nconns; i++)
> appendPQExpBuffer(&wait_query, ", %s", backend_pids[i]);
> appendPQExpBufferStr(&wait_query,
> ") "
>
> "AND holder.mode = ANY (CASE waiter.mode "
> "WHEN 'AccessShareLock' THEN ARRAY["
> "'AccessExclusiveLock'] "
> "WHEN 'RowShareLock' THEN ARRAY["
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'RowExclusiveLock' THEN ARRAY["
> "'ShareLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'ShareUpdateExclusiveLock' THEN ARRAY["
> "'ShareUpdateExclusiveLock',"
> "'ShareLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'ShareLock' THEN ARRAY["
> "'RowExclusiveLock',"
> "'ShareUpdateExclusiveLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'ShareRowExclusiveLock' THEN ARRAY["
> "'RowExclusiveLock',"
> "'ShareUpdateExclusiveLock',"
> "'ShareLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'ExclusiveLock' THEN ARRAY["
> "'RowShareLock',"
> "'RowExclusiveLock',"
> "'ShareUpdateExclusiveLock',"
> "'ShareLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] "
> "WHEN 'AccessExclusiveLock' THEN ARRAY["
> "'AccessShareLock',"
> "'RowShareLock',"
> "'RowExclusiveLock',"
> "'ShareUpdateExclusiveLock',"
> "'ShareLock',"
> "'ShareRowExclusiveLock',"
> "'ExclusiveLock',"
> "'AccessExclusiveLock'] END) "
>
> "AND holder.locktype IS NOT DISTINCT FROM waiter.locktype "
> "AND holder.database IS NOT DISTINCT FROM waiter.database "
> "AND holder.relation IS NOT DISTINCT FROM waiter.relation "
> "AND holder.page IS NOT DISTINCT FROM waiter.page "
> "AND holder.tuple IS NOT DISTINCT FROM waiter.tuple "
> "AND holder.virtualxid IS NOT DISTINCT FROM waiter.virtualxid "
> "AND holder.transactionid IS NOT DISTINCT FROM waiter.transactionid "
> "AND holder.classid IS NOT DISTINCT FROM waiter.classid "
> "AND holder.objid IS NOT DISTINCT FROM waiter.objid "
> "AND holder.objsubid IS NOT DISTINCT FROM waiter.objsubid ");
>
> This is way more knowledge than we (should) want a client to embed about
> which lock types block which others. What's worse, it's still wrong.
> The query will find cases where one of the test sessions *directly*
> blocks another one, but not cases where the blockage is indirect.
> For example, consider that A holds AccessShareLock, B is waiting for
> AccessExclusiveLock on the same object, and C is queued up behind B
> for another AccessShareLock. This query will not think that C is
> blocked, not even if B is part of the set of sessions of interest
> (because B will show the lock as not granted); but especially so if
> B is not part of the set.
>
> I think that such situations may not arise in the specific context that
> isolationtester says it's worried about, which is to disregard waits for
> locks held by autovacuum. But in general, you can't reliably tell who's
> blocking whom with a query like this.
>
> If isolationtester were the only market for this type of information,
> maybe it wouldn't be worth worrying about. But I'm pretty sure that
> there are a *lot* of monitoring applications out there that are trying
> to extract who-blocks-whom information from pg_locks. I hadn't realized
> before quite how painful it is to do that, even incorrectly.
>
> I propose that we should add a backend function that simplifies this
> type of query. The API that comes to mind is (name subject to
> bikeshedding)
>
> pg_blocking_pids(pid int) returns int[]
>
> defined to return NULL if the argument isn't the PID of any backend or
> that backend isn't waiting for a lock, and otherwise an array of the
> PIDs of the backends that are blocking it from getting the lock.
> I would compute the array as
>
> PIDs of backends already holding conflicting locks,
> plus PIDs of backends requesting conflicting locks that are
> ahead of this one in the lock's wait queue,
> plus PIDs of backends that block the latter group of PIDs
> (ie, are holding locks conflicting with their requests,
> or are awaiting such locks and are ahead of them in the queue)
>
> There would be some cases where this definition would be too expansive,
> ie we'd release the waiter after only some of the listed sessions had
> released their lock or request. (That could happen for instance if we
> concluded we had to move up the waiter's request to escape a deadlock.)
> But I think that it's better to err in that direction than to
> underestimate the set of relevant PIDs.
>
> In the isolationtester use-case, we'd get the right answer by testing
> whether this function's result has any overlap with the set of PIDs of
> test sessions, ie
>
> select pg_blocking_pids($1) && array[pid1, pid2, pid3, ...]

Sounds excellent.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company