From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 631F02CCAF for ; Fri, 19 Apr 2019 08:44:12 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wxdJEQMVP09l for ; Fri, 19 Apr 2019 08:44:12 -0400 (EDT) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id F16C42CCAC for ; Fri, 19 Apr 2019 08:44:11 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] [PATCH 05/10] Get rid of fiber_gc from txn_rollback Date: Fri, 19 Apr 2019 15:44:01 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Georgy Kirichenko Don't touch a fiber gc storage on a transaction rollback. This relaxes dependencies between fiber and transaction life cycles. Prerequisites: #1254 --- src/box/applier.cc | 9 ++++++--- src/box/box.cc | 24 +++++++++++++++++------- src/box/call.c | 16 ++++++++++++---- src/box/memtx_engine.c | 3 ++- src/box/txn.c | 22 +++++++++++----------- src/box/txn.h | 7 +++++-- src/box/vy_scheduler.c | 6 ++++-- 7 files changed, 57 insertions(+), 30 deletions(-) diff --git a/src/box/applier.cc b/src/box/applier.cc index 3b74e0f54..42b6efc4d 100644 --- a/src/box/applier.cc +++ b/src/box/applier.cc @@ -182,7 +182,8 @@ apply_initial_join_row(struct xrow_header *row) return -1; /* no access checks here - applier always works with admin privs */ if (space_apply_initial_join_row(space, &request)) { - txn_rollback(); + txn_rollback(txn); + fiber_gc(); return -1; } int rc = txn_commit(txn); @@ -418,7 +419,8 @@ applier_join(struct applier *applier) if (txn == NULL) diag_raise(); if (apply_row(&row) != 0) { - txn_rollback(); + txn_rollback(txn); + fiber_gc(); diag_raise(); } if (txn_commit(txn) != 0) @@ -621,7 +623,8 @@ applier_apply_tx(struct stailq *rows) return txn_commit(txn); rollback: - txn_rollback(); + txn_rollback(txn); + fiber_gc(); return -1; } diff --git a/src/box/box.cc b/src/box/box.cc index 54210474f..835a00c95 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -197,9 +197,12 @@ box_process_rw(struct request *request, struct space *space, if (txn_commit_stmt(txn, request) != 0) goto fail; + if (autocommit) { - if (txn_commit(txn) != 0) + if (txn_commit(txn) != 0) { + txn = NULL; goto fail; + } fiber_gc(); } @@ -212,8 +215,11 @@ box_process_rw(struct request *request, struct space *space, return 0; fail: - if (autocommit) - txn_rollback(); + if (autocommit) { + if (txn != NULL) + txn_rollback(txn); + fiber_gc(); + } if (tuple != NULL) tuple_unref(tuple); return -1; @@ -323,8 +329,10 @@ apply_wal_row(struct xstream *stream, struct xrow_header *row) if (txn == NULL || box_process_rw(&request, space, NULL) != 0 || txn_commit(txn) != 0) { say_error("error applying row: %s", request_str(&request)); - if (txn != NULL) - txn_rollback(); + if (txn != NULL) { + txn_rollback(txn); + fiber_gc(); + } diag_raise(); } } @@ -1342,7 +1350,8 @@ box_register_replica(uint32_t id, const struct tt_uuid *uuid) diag_raise(); if (boxk(IPROTO_INSERT, BOX_CLUSTER_ID, "[%u%s]", (unsigned) id, tt_uuid_str(uuid)) != 0) { - txn_rollback(); + txn_rollback(txn); + fiber_gc(); diag_raise(); } if (txn_commit(txn) != 0) @@ -1673,7 +1682,8 @@ box_set_replicaset_uuid(const struct tt_uuid *replicaset_uuid) /* Save replica set UUID in _schema */ if (boxk(IPROTO_INSERT, BOX_SCHEMA_ID, "[%s%s]", "cluster", tt_uuid_str(&uu))) { - txn_rollback(); + txn_rollback(txn); + fiber_gc(); diag_raise(); } if (txn_commit(txn) != 0) diff --git a/src/box/call.c b/src/box/call.c index b9750c5f3..4b5f155df 100644 --- a/src/box/call.c +++ b/src/box/call.c @@ -208,13 +208,17 @@ box_process_call(struct call_request *request, struct port *port) fiber_set_user(fiber(), orig_credentials); if (rc != 0) { - txn_rollback(); + if (in_txn() != NULL) + txn_rollback(in_txn()); + fiber_gc(); return -1; } if (in_txn()) { diag_set(ClientError, ER_FUNCTION_TX_ACTIVE); - txn_rollback(); + if (in_txn() != NULL) + txn_rollback(in_txn()); + fiber_gc(); return -1; } @@ -229,13 +233,17 @@ box_process_eval(struct call_request *request, struct port *port) if (access_check_universe(PRIV_X) != 0) return -1; if (box_lua_eval(request, port) != 0) { - txn_rollback(); + if (in_txn() != NULL) + txn_rollback(in_txn()); + fiber_gc(); return -1; } if (in_txn()) { diag_set(ClientError, ER_FUNCTION_TX_ACTIVE); - txn_rollback(); + if (in_txn() != NULL) + txn_rollback(in_txn()); + fiber_gc(); return -1; } diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index afa1739c6..cf2205c68 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -277,7 +277,8 @@ memtx_engine_recover_snapshot_row(struct memtx_engine *memtx, return -1; /* no access checks here - applier always works with admin privs */ if (space_apply_initial_join_row(space, &request) != 0) { - txn_rollback(); + txn_rollback(txn); + fiber_gc(); return -1; } int rc = txn_commit(txn); diff --git a/src/box/txn.c b/src/box/txn.c index 804962767..fdfffa144 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -347,7 +347,7 @@ txn_write_to_wal(struct txn *txn) if (res < 0) { /* Cascading rollback. */ - txn_rollback(); /* Perform our part of cascading rollback. */ + txn_rollback(txn); /* Perform our part of cascading rollback. */ /* * Move fiber to end of event loop to avoid * execution of any new requests before all @@ -396,7 +396,7 @@ txn_commit(struct txn *txn) if (txn->n_new_rows + txn->n_applier_rows > 0) { txn->signature = txn_write_to_wal(txn); if (txn->signature < 0) - goto fail; + return -1; } /* * The transaction is in the binary log. No action below @@ -424,7 +424,7 @@ txn_commit(struct txn *txn) txn_free(txn); return 0; fail: - txn_rollback(); + txn_rollback(txn); return -1; } @@ -439,11 +439,9 @@ txn_rollback_stmt() } void -txn_rollback() +txn_rollback(struct txn *txn) { - struct txn *txn = in_txn(); - if (txn == NULL) - return; + assert(txn == in_txn()); /* Rollback triggers must not throw. */ if (txn->has_triggers && trigger_run(&txn->on_rollback, txn) != 0) { @@ -458,8 +456,6 @@ txn_rollback() stailq_foreach_entry(stmt, &txn->stmts, next) txn_stmt_unref_tuples(stmt); - /** Free volatile txn memory. */ - fiber_gc(); fiber_set_txn(fiber(), NULL); txn_free(txn); } @@ -535,11 +531,14 @@ int box_txn_rollback() { struct txn *txn = in_txn(); + if (txn == NULL) + return 0; if (txn && txn->in_sub_stmt) { diag_set(ClientError, ER_ROLLBACK_IN_SUB_STMT); return -1; } - txn_rollback(); /* doesn't throw */ + txn_rollback(txn); /* doesn't throw */ + fiber_gc(); return 0; } @@ -617,6 +616,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 */ + } diff --git a/src/box/txn.h b/src/box/txn.h index 7f999b8e9..ae2d7b9a5 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -213,9 +213,12 @@ txn_begin(); int txn_commit(struct txn *txn); -/** Rollback a transaction, if any. */ +/** + * Rollback a transaction. + * @pre txn == in_txn() + */ void -txn_rollback(); +txn_rollback(struct txn *txn); /** * Roll back the transaction but keep the object around. diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c index fabb4bb48..1f6b30f4a 100644 --- a/src/box/vy_scheduler.c +++ b/src/box/vy_scheduler.c @@ -889,8 +889,10 @@ vy_deferred_delete_batch_process_f(struct cmsg *cmsg) for (int i = 0; i < batch->count; i++) { if (vy_deferred_delete_process_one(deferred_delete_space, pk->space_id, pk->mem_format, - &batch->stmt[i]) != 0) + &batch->stmt[i]) != 0) { + txn_rollback(txn); goto fail; + } } if (txn_commit(txn) != 0) @@ -900,7 +902,7 @@ vy_deferred_delete_batch_process_f(struct cmsg *cmsg) fail: batch->is_failed = true; diag_move(diag_get(), &batch->diag); - txn_rollback(); + fiber_gc(); } /** -- 2.21.0