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

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Mon, Jul 08, 2019 at 03:22:48PM +0300, Konstantin Osipov wrote:
> * 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

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.