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>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: slow queries over information schema.tables |
Date: | 2018-12-07 22:04:32 |
Message-ID: | 24823.1544220272@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Dec 6, 2018 at 12:50 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> No. You need to do AIM *after* obtaining the lock, else you still
>> have the race condition that you can execute a query on a table
>> without being aware of recent DDL on it.
> Huh? The call in relation_openrv_extended() happens before acquiring the lock.
Oh, I was looking at the wrong AIM call :-(. You're talking about this
one:
/*
* Check for shared-cache-inval messages before trying to open the
* relation. This is needed even if we already hold a lock on the
* relation, because GRANT/REVOKE are executed without taking any lock on
* the target relation, and we want to be sure we see current ACL
* information. We can skip this if asked for NoLock, on the assumption
* that such a call is not the first one in the current command, and so we
* should be reasonably up-to-date already. (XXX this all could stand to
* be redesigned, but for the moment we'll keep doing this like it's been
* done historically.)
*/
if (lockmode != NoLock)
AcceptInvalidationMessages();
which is indeed about as bletcherous as it could possibly be.
The ideal thing would be for GRANT/REVOKE to act more like other DDL,
but I'm not sure how we get to that point: REVOKE SELECT, at the very
least, would have to take AccessExclusiveLock so that it'd block SELECT.
People doing things like GRANT/REVOKE ON ALL TABLES would doubtless
complain about the greatly increased risk of deadlock from taking a
pile of AELs. And that still would only fix the problem for table
privileges, not privileges on other object types that we have no
consistent locking scheme for.
I can see the attraction of replacing these AIM calls (and, probably,
the one in AtStart_Cache) with a single call that occurs somewhere
during statement startup. Not sure exactly where that should be;
but there is certainly not anything principled about doing it in
relation_open_xxx().
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2018-12-07 23:05:40 | Re: pg_partition_tree crashes for a non-defined relation |
Previous Message | Peter Geoghegan | 2018-12-07 21:44:20 | Re: Thinking about EXPLAIN ALTER TABLE |