Re: Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-02 12:45:35 -0400, Robert Haas wrote:
> On Thu, Apr 27, 2017 at 1:52 AM, Andres Freund <andres@anarazel.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.

My concern is that this allows two XLOG_SEQ_LOG records in two sessions
to be interleaved in a bad manner:
S1:XLogInsert
S2:XLogInsert
S2:CatalogTupleUpdate
S1:CatalogTupleUpdate

which'd mean that S1's catalog state would apply to the XLOG_SEQ_LOG'ed
state from S2.

> 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.

That's pretty broken, yes.


> 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.

Right, I don't want that to happen - I think it means we need a proper
lock here, but Peter seems to be against that for reasons I don't
understand.  It's what Michael had suggested in:
http://archives.postgresql.org/message-id/CAB7nPqRev_wK4k39hQBpQZRQ17v29guxfobnnmTYT_-hUU67BA%40mail.gmail.com

- Andres