Re: Concurrent ALTER SEQUENCE RESTART Regression

On Thu, Apr 27, 2017 at 1:52 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:=
> In contrast to <v10, the actual change of the tuple is *not* happening
> with the page lock held. But now we do log XLOG_SEQ_LOG, then unlock
> the buffer, and then do a CatalogTupleUpdate(). How is that correct?
> And if so, why isn't there a honking large comment explaining why it is?

I can't actually understand this complaint. I think this code is
buggy, but for different reasons than you do. The page lock is held,
because read_seq_tuple() acquires it. And then we call heap_open()
with the buffer lock held?! I don't think that's a good idea in any
way - we shouldn't be doing complex operations that might acquire a
buffer lock while we're holding a buffer lock. But by the same token
surely we don't want to do CatalogUpdateIndexes() while holding the
buffer lock either; mutual exclusion needs to be managed at some
higher level, using, say, a heavyweight tuple lock.

Another thing that doesn't look so good about AlterSequence is that it
uses RangeVarGetRelid() instead of RangeVarGetsRelidExtended() with
RangeVarCallbackOwnsRelation or some derivative thereof. That of
course means you can obstruct access to a sequence you don't own.

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