From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 5 Mar 2019 10:43:33 +0300 From: Konstantin Osipov Subject: Re: [PATCH 4/4] vinyl: abort rw transactions when instance switches to ro Message-ID: <20190305074333.GE21955@chai> References: <60a021480834a54e71b5c5308581e8bdbdc3f7c0.1551713282.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <60a021480834a54e71b5c5308581e8bdbdc3f7c0.1551713282.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: georgy@tarantool.org, tarantool-patches@freelists.org List-ID: * 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? > void > +tx_manager_abort_writers_for_ro(struct tx_manager *xm) > +{ > + struct vy_tx *tx; > + rlist_foreach_entry(tx, &xm->writers, in_writers) { > + /* Remote transactions ignore ro flag. */ > + if (tx->state == VINYL_TX_READY && !tx->is_remote) > + tx->state = VINYL_TX_ABORT; > + } > +} I think we should retry applier transaction in case of abort conlfict. The error code for transaction conflict is distinguishable in the diagnostics area. This would help here and when conflicting with user transactions. Please feel free to do in a separate patch - or Georgy could do it in scope of his work on transactional applier. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov