From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 8 Jul 2019 19:41:41 +0300 From: Vladimir Davydov Subject: Re: [PATCH 4/5] memtx: fix txn_on_yield for DDL transactions Message-ID: <20190708164141.35f667dw7537syax@esperanza> References: <9fb58e54aee22309ba87fd2d6bed6bd658ab2e6d.1562357452.git.vdavydov.dev@gmail.com> <20190708122248.GC11062@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190708122248.GC11062@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Mon, Jul 08, 2019 at 03:22:48PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/07/05 23:27]: > > Memtx engine doesn't allow yielding inside a transaction. To achieve > > that, it installs fiber->on_yield trigger that aborts the current > > transaction (rolls it back, but leaves it be so that commit fails). > > > > There's an exception though - DDL statements are allowed to yield. > > This is required so as not to block the event loop while a new index > > is built or a space format is checked. Currently, we handle this > > exception by checking space id and omitting installation of the > > trigger for system spaces. This isn't entirely correct, because we > > may yield after a DDL statement is complete, in which case the > > transaction won't be aborted though it should: > > > > box.begin() > > box.space.my_space:create_index('my_index') > > fiber.sleep(0) -- doesn't abort the transaction! > > > > This patch fixes the problem by making the memtx engine install the > > on_yield trigger unconditionally, for all kinds of transactions, and > > instead explicitly disabling the trigger for yielding DDL operations. > > Nit: I think since we set up triggers in memtx_* methods, we > should clear them inside memtx_* subsystem as well. > > so it's not txn_*, it's memtx_{enable|disable}_yields Calling a memtx_* method from vinyl would look rather weird IMO. > > another way to do it is to clear fiber->txn key whenever > check_format /build_index functions yield. Basically these > functions run in the background, in a transaction that is > temporarily detached from the main fiber so should not pollute the > caller fiber key. I don't get it. Need to discuss f2f. > > this is perhaps an over engineering at this stage, but I still > would want you to consider this.