From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com> |
Cc: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | Removal of AcceptInvalidationMessages broke things |
Date: | 2012-09-19 17:17:17 |
Message-ID: | 528.1348075037@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I looked into bug #7557, which demonstrates a case where a session fails
to notice a just-committed change in table permissions. This is pretty
obviously due to a failure to read the sinval message notifying other
backends of the pg_class.relacl update. Some digging in the git history
says it got broken here:
commit 4240e429d0c2d889d0cda23c618f94e12c13ade7
Author: Robert Haas <rhaas(at)postgresql(dot)org>
Date: Fri Jul 8 22:19:30 2011 -0400
Try to acquire relation locks in RangeVarGetRelid.
In the previous coding, we would look up a relation in RangeVarGetRelid,
lock the resulting OID, and then AcceptInvalidationMessages(). While
this was sufficient to ensure that we noticed any changes to the
relation definition before building the relcache entry, it didn't
handle the possibility that the name we looked up no longer referenced
the same OID. This was particularly problematic in the case where a
table had been dropped and recreated: we'd latch on to the entry for
the old relation and fail later on. Now, we acquire the relation lock
inside RangeVarGetRelid, and retry the name lookup if we notice that
invalidation messages have been processed meanwhile. Many operations
that would previously have failed with an error in the presence of
concurrent DDL will now succeed.
because that commit removed this bit from relation_openrv:
- /*
- * Check for shared-cache-inval messages before trying to open the
- * relation. This is needed to cover the case where the name identifies a
- * rel that has been dropped and recreated since the start of our
- * transaction: if we don't flush the old syscache entry then we'll latch
- * onto that entry and suffer an error when we do RelationIdGetRelation.
- * Note that relation_open does not need to do this, since a relation's
- * OID never changes.
- *
- * We skip this if asked for NoLock, on the assumption that the caller has
- * already ensured some appropriate lock is held.
- */
- if (lockmode != NoLock)
- AcceptInvalidationMessages();
and there are no other places where a transaction will do
AcceptInvalidationMessages if it thinks it already holds lock on the
tables it's working with.
Since we have only a few hours before 9.2.1 is due to wrap, my
inclination for a band-aid fix is to put back that code. There might be
some more elegant answer, but we haven't got time to find it now.
Comments?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2012-09-19 17:26:37 | Re: Invalid optimization of VOLATILE function in WHERE clause? |
Previous Message | Robert Haas | 2012-09-19 16:55:26 | Re: Invalid optimization of VOLATILE function in WHERE clause? |