From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 429EB44643B for ; Tue, 8 Sep 2020 13:22:14 +0300 (MSK) Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1kFalR-0006tc-Ly for tarantool-patches@dev.tarantool.org; Tue, 08 Sep 2020 13:22:14 +0300 From: Aleksandr Lyapunov Date: Tue, 8 Sep 2020 13:22:03 +0300 Message-Id: <1599560532-27089-4-git-send-email-alyapunov@tarantool.org> In-Reply-To: <1599560532-27089-1-git-send-email-alyapunov@tarantool.org> References: <1599560532-27089-1-git-send-email-alyapunov@tarantool.org> Subject: [Tarantool-patches] [PATCH v4 03/12] txm: save does_require_old_tuple flag in txn_stmt List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org That flag is needed for transactional conflict manager - if any other transaction commits a replacement of old_tuple before current one and the flag is set - the current transaction will be aborted. For example REPLACE just replaces a key, no matter what tuple lays in the index and thus does_require_old_tuple = false. In contrast, UPDATE makes new tuple using old_tuple and thus the statement will require old_tuple (does_require_old_tuple = true). INSERT also does_require_old_tuple = true because it requires old_tuple to be NULL. Part of #4897 --- src/box/memtx_space.c | 17 +++++++++++++++++ src/box/txn.c | 53 ++++++++++++++++++++++++++++++++++----------------- src/box/txn.h | 13 +++++++++++++ 3 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c index d30ce44..7624130 100644 --- a/src/box/memtx_space.c +++ b/src/box/memtx_space.c @@ -316,6 +316,10 @@ memtx_space_execute_replace(struct space *space, struct txn *txn, if (stmt->new_tuple == NULL) return -1; tuple_ref(stmt->new_tuple); + + if (mode == DUP_INSERT) + stmt->does_require_old_tuple = true; + if (memtx_space->replace(space, NULL, stmt->new_tuple, mode, &stmt->old_tuple) != 0) return -1; @@ -342,6 +346,13 @@ memtx_space_execute_delete(struct space *space, struct txn *txn, struct tuple *old_tuple; if (index_get(pk, key, part_count, &old_tuple) != 0) return -1; + + /* + * We have to delete exactly old_tuple just because we return it as + * a result. + */ + stmt->does_require_old_tuple = true; + if (old_tuple != NULL && memtx_space->replace(space, old_tuple, NULL, DUP_REPLACE_OR_INSERT, &stmt->old_tuple) != 0) @@ -390,6 +401,9 @@ memtx_space_execute_update(struct space *space, struct txn *txn, if (stmt->new_tuple == NULL) return -1; tuple_ref(stmt->new_tuple); + + stmt->does_require_old_tuple = true; + if (memtx_space->replace(space, old_tuple, stmt->new_tuple, DUP_REPLACE, &stmt->old_tuple) != 0) return -1; @@ -496,6 +510,9 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn, stmt->new_tuple = NULL; } } + + stmt->does_require_old_tuple = true; + /* * It's OK to use DUP_REPLACE_OR_INSERT: we don't risk * inserting a new tuple if the old one exists, since diff --git a/src/box/txn.c b/src/box/txn.c index 093808a..d9fbb51 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -109,6 +109,7 @@ txn_stmt_new(struct region *region) stmt->engine_savepoint = NULL; stmt->row = NULL; stmt->has_triggers = false; + stmt->does_require_old_tuple = false; return stmt; } @@ -351,27 +352,43 @@ txn_commit_stmt(struct txn *txn, struct request *request) * - perhaps we should run triggers even for deletes which * doesn't find any rows */ - if (stmt->space != NULL && !rlist_empty(&stmt->space->on_replace) && - stmt->space->run_triggers && (stmt->old_tuple || stmt->new_tuple)) { - int rc = 0; - if(!space_is_temporary(stmt->space)) { - rc = trigger_run(&stmt->space->on_replace, txn); - } else { + if (stmt->space != NULL && stmt->space->run_triggers && + (stmt->old_tuple || stmt->new_tuple)) { + if (!rlist_empty(&stmt->space->before_replace)) { /* - * There is no row attached to txn_stmt for - * temporary spaces, since DML operations on them - * are not written to WAL. Fake a row to pass operation - * type to lua on_replace triggers. + * Triggers see old_tuple and that tuple + * must remain the same */ - assert(stmt->row == NULL); - struct xrow_header temp_header; - temp_header.type = request->type; - stmt->row = &temp_header; - rc = trigger_run(&stmt->space->on_replace, txn); - stmt->row = NULL; + stmt->does_require_old_tuple = true; + } + if (!rlist_empty(&stmt->space->on_replace)) { + /* + * Triggers see old_tuple and that tuple + * must remain the same + */ + stmt->does_require_old_tuple = true; + + int rc = 0; + if(!space_is_temporary(stmt->space)) { + rc = trigger_run(&stmt->space->on_replace, txn); + } else { + /* + * There is no row attached to txn_stmt for + * temporary spaces, since DML operations on + * them are not written to WAL. + * Fake a row to pass operation type to lua + * on_replace triggers. + */ + assert(stmt->row == NULL); + struct xrow_header temp_header; + temp_header.type = request->type; + stmt->row = &temp_header; + rc = trigger_run(&stmt->space->on_replace, txn); + stmt->row = NULL; + } + if (rc != 0) + goto fail; } - if (rc != 0) - goto fail; } --txn->in_sub_stmt; return 0; diff --git a/src/box/txn.h b/src/box/txn.h index 35e1ab9..007284f 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -175,6 +175,19 @@ struct txn_stmt { struct xrow_header *row; /** on_commit and/or on_rollback list is not empty. */ bool has_triggers; + /** + * Whether the stmt requires to replace exactly old_tuple (member). + * That flag is needed for transactional conflict manager - if any + * other transaction commits a replacement of old_tuple before current + * one and the flag is set - the current transaction will be aborted. + * For example REPLACE just replaces a key, no matter what tuple + * lays in the index and thus does_require_old_tuple = false. + * In contrast, UPDATE makes new tuple using old_tuple and thus + * the statement will require old_tuple (does_require_old_tuple = true). + * INSERT also does_require_old_tuple = true because it requires + * old_tuple to be NULL. + */ + bool does_require_old_tuple; /** Commit/rollback triggers associated with this statement. */ struct rlist on_commit; struct rlist on_rollback; -- 2.7.4