Re: [PATCH 4/4] vinyl: abort rw transactions when instance switches to ro

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Tue, Mar 05, 2019 at 10:43:33AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/03/05 10:25]:
> > A Vinyl transaction may yield while having a non-empty write set. This
> > opens a time window for the instance to switch to read-only mode. Since
> > we check ro flag only before executing a DML request, the transaction
> > would successfully commit in such a case, breaking the assumption that
> > no writes are possible on an instance after box.cfg{read_only=true}
> > returns. In particular, this breaks master-replica switching logic.
> > 
> > Fix this by aborting all local rw transactions before switching to
> > read-only mode. Note, remote rw transactions must not be aborted,
> > because they ignore ro flag.
> > 
> > Closes #4016
> 
> OK to push, a few comments below.
> 
> > -	double timeout = (current_session()->type != SESSION_TYPE_APPLIER ?
> > -			  env->timeout : TIMEOUT_INFINITY);
> > +	double timeout = (tx->is_remote ? TIMEOUT_INFINITY : env->timeout);
> 
> is_remote is a vague name. A net.box connection is remote as well.
> Why not simply have tx->session_type or
> tx->session_type_is_applier or tx->is_session_type_applier? 

Right. Renamed to is_applier_session and pushed all the four patches
to 2.1 and 1.10.