From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 8 Jul 2019 11:41:18 +0300 From: Vladimir Davydov Subject: Re: [PATCH 6/6] Replace schema lock with fine-grained locking Message-ID: <20190708084118.4nqq6zko2ypmstc4@esperanza> References: <20190703193541.GH17318@atlas> <20190703195605.dhuoxv7xrxqzklug@esperanza> <20190704080909.GB24820@atlas> <20190704170620.s5jre26y4crm4xh4@esperanza> <20190708074053.GA3352@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190708074053.GA3352@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Mon, Jul 08, 2019 at 10:40:53AM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/07/04 20:10]: > > Come to think about this commit, it still raises a lot of > questions. > > What is it taken for entire alter_space_do while it protects alter_space_prepare > only? Simply because it's easier to write the code that way. Since we only care about yields, the lock has no effect if no operation yields. OTOH taking the lock early makes alter_space_do fail early in case the space happens to be busy. > > Why is it necessary to protect the yields from prepare, but not > from WAL? Once we submitted a request to WAL, we don't really care about concurrent DDL modifying the space as we are done working with it. We only care that the space doesn't go away until we run the commit or rollback trigger. And it can't go away, because other DDL operations will wait for WAL as well, which orders them to happen after us. > > Why is not not taken when a new space is created? What happens > when box.schema.space:create() is run concurrently from two > fibers, would that work without a lock? Why? We don't need to take the lock when a space is created or dropped, because those operations never yield. Concurrent requests are ordered by the WAL just like any other concurrent DDL. > > These questions should at least be addressed in the comments to > the lock class. Urgh... Isn't it obvious? > > And since the lock is wrapped around long yielding operations, > perhaps it should be taken by objects which do these operations, > like CheckSpaceFormat and CreateIndex (it is easy to grant the > lock to a fiber which already owns it, so each object could be > taken its own lock)? I think of locks as protecting objects, not pieces of code. This particular lock protects the space struct from getting modified by a concurrent request. I don't see any benefit resulting from taking it deeper in the call stack - it would only spread the logic between different functions.