From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <david.rowley@2ndquadrant.com>
cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>,
        Julien Rouhaud <rjuju123@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Andres Freund <andres@anarazel.de>,
        Thomas Munro <thomas.munro@gmail.com>,
        Amit Kapila <amit.kapila16@gmail.com>,
        Haribabu Kommi <kommi.haribabu@gmail.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Rushabh Lathia <rushabh.lathia@gmail.com>
Subject: Re: Inadequate executor locking of indexes
In-reply-to: 
 <CAKJS1f-sqy=oxNLkVyJO9WsPCyVFBXTvzA9nEBmPgQESu84x4Q@mail.gmail.com>
References: <19465.1541636036@sss.pgh.pa.us>
 <31575bce-fbcd-530d-1f97-455c7959cd73@lab.ntt.co.jp>
 <CAKJS1f8FH6exr6VjAAKNwZBybUzZgPVigCW-k=maENCe-Se-bQ@mail.gmail.com>
 <CAKJS1f9m03-JWx1rBzg4jEoVDKCQusGCxiGs0CdH9d-_FYBbnA@mail.gmail.com>
 <CAOBaU_YWbrQgta8qT5fSM0BoJWJL3keSDwExjaiki3LLOY=9KQ@mail.gmail.com>
 <CAKJS1f-sUgU2BLwfyv9ZQCR5cVbsOSBKDk--gCr6WGhGfJe2pQ@mail.gmail.com>
 <CAOBaU_b-00SqV8soUizOp-OJd8Z+pCXrFg8LAwE1vq15rxi+Cg@mail.gmail.com>
 <CAKJS1f-g6fHxGGyFmjw=H+D7-esC6ECjDvvVYHK1oQHcsg2-tQ@mail.gmail.com>
 <CAKJS1f_ZaECEN+GFtfrepe18ovoqsKNs8YML1BwsnZHr3fdzKQ@mail.gmail.com>
 <a865d032-920f-8bd7-0392-b22d4dea2f3a@lab.ntt.co.jp>
 <CAKJS1f8zy30SueM0WQkRuLLUEPvbys5e5NZaKGKrdSEC-Eb+OQ@mail.gmail.com>
 <CAKJS1f9GjRs61WEswX+SXkXD7fi33opZk8wBHfB1pR1ttfJ=_A@mail.gmail.com>
 <995.1554224619@sss.pgh.pa.us>
 <CAKJS1f-Fq5ywNWU8x2=yZ1X1K4PJs+azV8eHYZ39T+ZTEzUiAw@mail.gmail.com>
 <c01cd66e-44f5-71c4-1e28-9cf25605b8f5@lab.ntt.co.jp>
 <CAKJS1f-zzk9zWnG6Xurn83C1a1Hm2g7Q-XiZqYtwu=3NKdJwCw@mail.gmail.!
 com> <dec0b0a4-9001-c613-fb02-449111b7d327@lab.ntt.co.jp>
 <CAKJS1f9JLYyTLb==1gRtFe2QU7pMWnqhug_bt03aCAEUT7Gerg@mail.gmail.com>
 <CAKJS1f-sqy=oxNLkVyJO9WsPCyVFBXTvzA9nEBmPgQESu84x4Q@mail.gmail.com>
Comments: In-reply-to David Rowley <david.rowley@2ndquadrant.com>
	message dated "Fri, 05 Apr 2019 02:25:21 +1300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <30374.1554406015.1@sss.pgh.pa.us>
Date: Thu, 04 Apr 2019 15:26:55 -0400
Message-ID: <30375.1554406015@sss.pgh.pa.us>
Precedence: bulk

David Rowley <david.rowley@2ndquadrant.com> writes:
> Wrong patch.  Here's what I meant to send.

Pushed with some minor tweaking, mostly comments.

Some notes:

* We now have a general convention that queries always take the same lock
type on indexes as on their parent tables, but that convention is not
respected by index DDL.  I'm not sure if there is any additional cleanup
that should be done there.  The DDL code intends to block concurrent
execution of other DDL on the same index, in most cases, so it may be
fine.  At the very least, the different lock levels for those cases are
clearly intentional, while I don't think that was true for DML.

* I dropped the extra assertion you'd added to infer_arbiter_indexes,
as it didn't seem necessary or appropriate.  That function's just
interested in inspecting the index, so it doesn't need to assume anything
about how strong the lock is.  I think the comment that was there was
just trying to justify the shortcut of hard-wiring the lock level as
RowExclusiveLock.

* I went ahead and changed gin_clean_pending_list() to take
RowExclusiveLock not AccessShareLock on its target index.  I'm not quite
sure that AccessShareLock is an actual bug there; it may be that GIN's
internal conventions are such that that's safe.  But it sure seemed darn
peculiar, and at risk of causing future problems.

			regards, tom lane