[tarantool-patches] Re: [PATCH] Do not enable commit if read_only = true

[Konstantin Osipov]

* Георгий Кириченко <georgy at tarantool.org> [19/03/03 23:50]:
> On Thursday, February 28, 2019 1:25:30 PM MSK Konstantin Osipov wrote:
> > * Vladimir Davydov <vdavydov.dev at gmail.com> [19/02/27 12:26]:
> > > On Wed, Feb 27, 2019 at 10:36:00AM +0300, Georgy Kirichenko wrote:
> > > > Disable commit if server is in read only mode.
> > > 
> > > The commit message is very poor. Please elaborate why this is important.
> > 
> > Having thought about it we should go over all in-flight
> > transactions in vinyl and abort them.
> > 
> > We already use this approach on DDL.
> > We could make it more general.
> I did a little investigation and found that current implementation does not 
> allow us do reach expected behavior using vinyl aborts because of
> * transaction placed in the write set only after uniqueness
> check - this leads 
> to a race if read only was set during this check.

tx_manager_abort_writers() looks for a single lsm only anyway. In
case of read-only you need to abort all write transactions
against all spaces. 

You have two broad options with this problem: fix the existing
infrastructure or begin building a new one. 

Adding an extra check to txn_commit() is neither. You will have
two checks for read_only, but your ddl will continue to be broken.

For example, the problem you mention about vinyl write
transactions being added to tx_manager->writers after yield also
affects vinyl ddl. By adding an extra read_only check to
txn_commit() you won't fix it.

So I think we should first consider fixing tx_manager->writers list, so
that transactions end up in this list before yield, and
implementing vy_tx_about_writers() for all write transactions. I
asked Vova to look into this since this is vinyl domain and a bit
tricky one.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov