From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 20 Jun 2019 16:03:52 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] [PATCH v4 4/9] txn: get rid of fiber_gc from txn_rollback Message-ID: <20190620130352.q763jjpvkusujtq4@esperanza> References: <0eeb7c2b0e45bfae1cba4488292c7d6fc7e69fa4.1560978655.git.georgy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0eeb7c2b0e45bfae1cba4488292c7d6fc7e69fa4.1560978655.git.georgy@tarantool.org> To: Georgy Kirichenko Cc: tarantool-patches@freelists.org List-ID: On Thu, Jun 20, 2019 at 12:23:11AM +0300, Georgy Kirichenko wrote: > Don't touch a fiber gc storage on a transaction rollback explicitly. > This relaxes dependencies between fiber and transaction life cycles. > > Prerequisites: #1254 > --- > src/box/applier.cc | 8 +++++--- > src/box/box.cc | 14 +++++++++----- > src/box/call.c | 22 ++++++++++++++++------ > src/box/memtx_engine.c | 3 ++- > src/box/txn.c | 35 +++++++++++++++++------------------ > src/box/txn.h | 8 ++++++-- > src/box/vy_scheduler.c | 10 +++++++--- > 7 files changed, 62 insertions(+), 38 deletions(-) > > diff --git a/src/box/txn.c b/src/box/txn.c > index 39b1ed773..21f7e98b4 100644 > --- a/src/box/txn.c > +++ b/src/box/txn.c > @@ -168,6 +168,10 @@ txn_new() > inline static void > txn_free(struct txn *txn) > { > + struct txn_stmt *stmt; > + stailq_foreach_entry(stmt, &txn->stmts, next) > + txn_stmt_unref_tuples(stmt); > + Clearly, this doesn't belong here. Should be a part of the previous patch? > /* Truncate region up to struct txn size. */ > region_truncate(&txn->region, sizeof(struct txn)); > stailq_add(&txn_cache, &txn->in_txn_cache); > @@ -636,6 +634,7 @@ txn_on_stop(struct trigger *trigger, void *event) > { > (void) trigger; > (void) event; > - txn_rollback(); /* doesn't yield or fail */ > + txn_rollback(in_txn()); /* doesn't yield or fail */ > + Extra new line. > } > > diff --git a/src/box/txn.h b/src/box/txn.h > index 5a66f8e53..569978ce9 100644 > --- a/src/box/txn.h > +++ b/src/box/txn.h > @@ -267,6 +270,7 @@ txn_on_rollback(struct txn *txn, struct trigger *trigger) > > /** > * Start a new statement. > + * Return a new statement or NULL in case of error. Again, looks like a leftover from the prevoius patch. Other than that the patch looks fine to me. I would rather remove fiber_gc() altogether, but as Kostja pointed out, it's too risky so fine - let it be. > */ > struct txn_stmt * > txn_begin_stmt(struct txn *txn, struct space *space);