From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 8 Jul 2019 15:14:08 +0300 From: Konstantin Osipov Subject: Re: [PATCH 2/5] txn: run on_rollback triggers on txn_abort Message-ID: <20190708121408.GA11062@atlas> References: <20190708093201.GC8512@atlas> <20190708095746.uay7r44jxhhk5xeh@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190708095746.uay7r44jxhhk5xeh@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [19/07/08 13:02]: > > This manipulation should be in txn_abort(), not in > > txn_run_triggers(). It's txn_abort(). > > But this function is also called from completion callback, where it has > to set the txn as well. That's why I put it there, otherwise we would > have to set/restore txn context in txn_complete as well. I'm not really > against it - just pointint it out. I'll prepare a patch that does that, > see how it looks. Yes, so, basically, txn_begin(), txn_abort(), txn_complete() should manage the fiber key. > > > > fiber_set_txn(fiber(), txn); > > > /* Rollback triggers must not throw. */ > > > if (trigger_run(trigger, txn) != 0) { > > > @@ -357,7 +358,7 @@ txn_run_triggers(struct txn *txn, struct rlist *trigger) > > > unreachable(); > > > panic("commit/rollback trigger failed"); > > > } > > > - fiber_set_txn(fiber(), NULL); > > > + fiber_set_txn(fiber(), old_txn); > > > > Ideally we should never need to restore old_txn. All transaction > > statements, like txn_begin() or txn_abort() should set the txn, > > and whenever the transaction yields, the txn should be cleared. > > But we do want the transaction to remain attached to the fiber once > it resumes its execution so that we can raise an error on 'commit'. > Actually, we used to clear txn on yield, but then it was reworked > to make 'commit' more user-friendly. I don't get it. When a transaction is resumed after a yield, it has to set its key again anyway. Basically, transaction == fiber is no longer true, and each time a transaction starts running it should update its key. Better yet, let's kill the transaction key in the fiber altogether and pass the txn around by value. -- Konstantin Osipov, Moscow, Russia