Thread

Re: Make relation_openrv atomic wrt DDL

Robert Haas <robertmhaas@gmail.com> — 2011-07-06T19:06:40Z

On Sun, Jun 19, 2011 at 11:42 PM, Noah Misch <noah@leadboat.com> wrote:
> On Sun, Jun 19, 2011 at 11:44:35PM +0100, Greg Stark wrote:
>> So I was the victim assigned to review this patch.
>
> Thanks for doing so.

This discussion seems to have died off.  Let's see if we can drive
this forward to some conclusion.

I took a look at this patch and found that it had bit-rotted slightly.
 I am attaching a rebased version.

Maybe this is a stupid idea, but what about changing the logic so
that, if we get back InvalidOid, we AcceptInvalidationMessages() and
retry if the counter has advanced?  ISTM that might cover the example
you mentioned in your last post, where we fail to detect a relation
that has come into existence since our last call to
AcceptInvalidationMessages().  It would cost an extra
AcceptInvalidationMessages() only in the case where we haven't found
the relation, which (a) seems like a good time to worry about whether
we're missing something, since users generally try not to reference
nonexistent tables and (b) should be rare enough to be ignorable from
a performance perspective.

In the department of minor nitpicking, why not use a 64-bit counter
for SharedInvalidMessageCounter?  Then we don't have to think very
hard about whether overflow can ever pose a problem.

It strikes me that, even with this patch, there is a fair amount of
room for wonky behavior.  For example, as your comment notes, if
search_path = foo, bar, and we've previously referenced "x", getting
"bar.x", the creation of "foo.x" will generate invalidation messages,
but a subsequent reference - within the same transaction - to "x" will
not cause us to read them.  It would be nice to
AcceptInvalidationMessages() unconditionally at the beginning of
RangeVarGetRelid() [and then redo as necessary to get a stable
answer], but that might have some performance consequence for
transactions that repeatedly read the same tables.

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