From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 8 Jul 2019 15:22:48 +0300 From: Konstantin Osipov Subject: Re: [PATCH 4/5] memtx: fix txn_on_yield for DDL transactions Message-ID: <20190708122248.GC11062@atlas> References: <9fb58e54aee22309ba87fd2d6bed6bd658ab2e6d.1562357452.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9fb58e54aee22309ba87fd2d6bed6bd658ab2e6d.1562357452.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * 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 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. this is perhaps an over engineering at this stage, but I still would want you to consider this. -- Konstantin Osipov, Moscow, Russia