From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 5 Mar 2019 11:35:27 +0300 From: Vladimir Davydov Subject: Re: [PATCH 4/4] vinyl: abort rw transactions when instance switches to ro Message-ID: <20190305083527.ialkb6bgfewuwu3a@esperanza> References: <60a021480834a54e71b5c5308581e8bdbdc3f7c0.1551713282.git.vdavydov.dev@gmail.com> <20190305074333.GE21955@chai> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190305074333.GE21955@chai> To: Konstantin Osipov Cc: georgy@tarantool.org, tarantool-patches@freelists.org List-ID: On Tue, Mar 05, 2019 at 10:43:33AM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [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.