Re: Inadequate executor locking of indexes

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
Subject: Re: Inadequate executor locking of indexes
Date: 2019-03-13 09:44:58
Message-ID: a865d032-920f-8bd7-0392-b22d4dea2f3a@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi David,

On 2019/03/13 10:38, David Rowley wrote:
> I had another go at this patch and fixed the problem by just setting
> the idxlockmode inside the planner just before the call to
> expand_inherited_tables(). This allows the lockmode to be copied into
> child RTEs.

I have one question about the relation between idxlockmode and
rellockmode? From skimming the patch, it appears that they're almost
always set to the same value. If so, why not use rellockmode for index
locking too?

> #2 would not address Tom's mention of there possibly being some way to
> have the index scan node initialise before the modify table node
> (currently we pass NoLock for indexes belonging to result rels in the
> index scan nodes). I can't quite imagine at the moment how that's
> possible, but maybe my imagination is just not good enough. We could
> fix that by passing RowExclusiveLock instead of NoLock. It just means
> we'd discover the lock already exists in the local lock table...
> unless of course there is a case where the index scan gets initialised
> before modify table is.

Tom gave an example upthread that looked like this:

explain (costs off) with x1 as materialized (select * from foo where a =
1), x2 as (update foo set a = a where a = 1 returning *) select * from x1,
x2 where x1.a = x2.a;
QUERY PLAN
────────────────────────────────────────────────────
Hash Join
Hash Cond: (x1.a = x2.a)
CTE x1
-> Bitmap Heap Scan on foo
Recheck Cond: (a = 1)
-> Bitmap Index Scan on foo_a_idx
Index Cond: (a = 1)
CTE x2
-> Update on foo foo_1
-> Bitmap Heap Scan on foo foo_1
Recheck Cond: (a = 1)
-> Bitmap Index Scan on foo_a_idx
Index Cond: (a = 1)
-> CTE Scan on x1
-> Hash
-> CTE Scan on x2
(16 rows)

When InitPlan() invokes ExecInitNode on the subplans of x1 and x2, it does
so in that order. So, ExecInitBitmapIndexScan for x1 is called before
ExecInitModifyTable for x2.

But if finalize_lockmodes() in your patch set lockmodes correctly,
ExecInitBitmapIndexScan() called for x1 ought to lock the index with
RowExclusiveLock, no?

Thanks,
Amit

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-03-13 09:49:23 Re: CPU costs of random_zipfian in pgbench
Previous Message Fabien COELHO 2019-03-13 09:44:03 Re: Offline enabling/disabling of data checksums