lock level for DETACH PARTITION looks sketchy

Поиск
Список
Период
Сортировка
От Robert Haas
Тема lock level for DETACH PARTITION looks sketchy
Дата
Msg-id CA+TgmoYruJQ+2qnFLtF1xQtr71pdwgfxy3Ziy-TxV28M6pEmyA@mail.gmail.com
обсуждение исходный текст
Ответы Re: lock level for DETACH PARTITION looks sketchy  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
While working on the problem of lowering lock levels for ATTACH
PARTITION and DETACH PARTITION, I discovered that DETACH PARTITION
takes only AccessShareLock on the table being detached, which I think
is not good.  It seems to me that at a minimum it needs to take a
self-exclusive lock, because otherwise you can get things like this,
which we ordinarily try to avoid:

S1: BEGIN;
S1: ALTER TABLE tab DETACH PARTITION tab2;
S2: alter table tab2 set (fillfactor = 90);
S1: COMMIT;
S2: ERROR:  tuple concurrently updated

I'm not 100% certain that ShareUpdateExclusiveLock is actually a
strong enough lock level, but I think that the right answer can't be
any weaker than that.  So I propose to increase the level to that
value and back-patch.  This looks to be a bug in the original table
partitioning commit (f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63).

I noticed another thing that looks odd as well.  ATExecDetachPartition
does this:

        idx = index_open(idxid, AccessExclusiveLock);
        IndexSetParentIndex(idx, InvalidOid);
        update_relispartition(classRel, idxid, false);
        relation_close(idx, AccessExclusiveLock);

That last line is unlike what we do nearly everywhere else in that it
releases a lock on a user relation before commit time. I don't know
whether that has any bad consequences, but it means that some other
backend could obtain a lock on that relation before we've queued any
invalidation messages associated with the change, which seems possibly
dangerous.

A minor nitpick is that the last like there should probably use
index_close() to match the index_open() a few lines above.  That code
happens to be identical to the code for relation_open(), but it's
probably better to use the matching function.

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


В списке pgsql-hackers по дате отправления:

Предыдущее
От: David Fetter
Дата:
Сообщение: Re: What to name the current heap after pluggable storage / what torename?
Следующее
От: Robert Haas
Дата:
Сообщение: Re: What to name the current heap after pluggable storage / what to rename?