From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Date: Fri, 12 Jul 2019 18:16:14 +0300 From: Konstantin Osipov Subject: Re: [PATCH v2 1/3] memtx: fix txn_on_yield for DDL transactions Message-ID: <20190712151614.GC8020@atlas> References: <20190710203439.GN5619@atlas> <20190712145419.2mesthmxtxxnt32l@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190712145419.2mesthmxtxxnt32l@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [19/07/12 17:57]: This is a very nice patch. Thank you for addressing my comments. The bit fields is going to be trivial, so go ahead and don't postpone it - it doesn't need a review. One more comment below. > diff --git a/src/box/vinyl.c b/src/box/vinyl.c > index e0de65d0..7635c84c 100644 > --- a/src/box/vinyl.c > +++ b/src/box/vinyl.c > @@ -1093,6 +1093,7 @@ static int > vinyl_space_check_format(struct space *space, struct tuple_format *format) > { > struct vy_env *env = vy_env(space->engine); > + struct txn *txn = in_txn(); > > /* > * If this is local recovery, the space was checked before > @@ -1121,6 +1122,8 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format) > bool need_wal_sync; > tx_manager_abort_writers_for_ddl(env->xm, space, &need_wal_sync); > > + txn_can_yield(txn, true); I keep forgetting how this can happen, and then remember it's a *memtx* transaction. Please add this comment: /* * Tarantool doesn't allow multi-engine transactions, and DDL * system tables are memtx tables. Memtx transactions, * generally, can't yield. * So here we're in the middle of a *memtx* transaction. We don't * start a hidden vinyl transaction for DDL either, to avoid its * overhead. But some long DDL operations can yield, like * checking a format or building an index. * Unless we switch off memtx yield rollback triggers, such * yield leads to memtx transaction rollback. It is safe to switch * the trigger off though: it protects subsequent memtx * transactions from reading a dirty state, and at this phase * vinyl DDL does not change the data * dictionary, so there is * no dirty state that can be observed. */ -- Konstantin Osipov, Moscow, Russia