Re: [PATCH 4/5] memtx: fix txn_on_yield for DDL transactions

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@gmail.com> [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