Re: Concurrent CREATE TABLE/DROP SCHEMA leaves inconsistent leftovers

On Fri, Jan 13, 2012 at 2:05 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Nov 29, 2011 at 10:10 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>
>> I have plans to try to improve this, but it's one of those things that
>> I care about more than the people who write the checks do, so it
>> hasn't quite gotten to the top of the priority list yet.
>
> All right... I have a patch that I think fixes this, at least so far
> as relations are concerned.  I rewrote
> RangeVarGetAndCheckCreationNamespace() extensively, did surgery on its
> callers, and then modified CREATE OR REPLACE VIEW and ALTER <relkind>
> .. SET SCHEMA to make use of it rather than doing things as they did
> before.
>
> So this patch prevents (1) concurrently dropping a schema in which a
> new relation is being created, (2) concurrently dropping a schema into
> which an existing relation is being moved, and (3) using CREATE OR
> REPLACE VIEW to attempt to obtain a lock on a relation you don't own
> (the command would eventually fail, of course, but if there were, say,
> an outstanding AccessShareLock on the relation, you'd queue up for the
> lock and thus prevent any further locks from being granted, despite
> your lack of any rights on the target).

The patch looks ok, though I wonder if we could have a way to release
the lock on namespace much before the end of transaction. Since the
lock is held all the time, now the possibility of dead lock is bigger.
Say,

-- tx1
BEGIN;
SELECT * FROM s.x;
DROP SCHEMA t;

-- tx2
BEGIN;
SELECT * FROM t.y;
DROP SCHEMA s;

I know it's a limited situation, though. Maybe the right way would be
to check the namespace at the end of the transaction if any object is
created, rather than locking it.

> It doesn't fix any of the non-relation object types.  That would be
> nice to do, but I think it's material for a separate patch.

I took a quick look under src/include/catalog and the objects that
have namespace are

- collation
- constraint
- conversion
- extension
- operator
- operator class
- operator family
- function (proc)
- ts_config
- ts_dict
- ts_parser
- ts_template
- (what's missing?)

I agree with you that it's not worth doing everything, but function is
nice to have. I don't mind if we don't have it with the other objects.

Thanks,
--
Hitoshi Harada