[Tarantool-patches] [PATCH v3 07/13] txm: save does_require_old_tuple flag in txn_stmt
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Jul 16 03:09:04 MSK 2020
Thanks for the patch!
See 3 comments below.
On 15.07.2020 15:55, Aleksandr Lyapunov wrote:
> That flag is needed for transactional conflict manager - if any
> other transaction replaces 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 | 3 +++
> src/box/txn.h | 13 +++++++++++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 8452ab4..e48ed3a 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;
1. Yeah, now I understand. NULL is also considered a tuple in txm.
Would be nice to have a comment here about it. In other places too,
btw. This is not so trivial, and it won't harm to write some
explanations.
> +
> if (memtx_space->replace(space, NULL, stmt->new_tuple,
> mode, &stmt->old_tuple) != 0)
> return -1;
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 0372047..d254edb 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -360,6 +361,8 @@ txn_commit_stmt(struct txn *txn, struct request *request)
> */
> if (stmt->space != NULL && !rlist_empty(&stmt->space->on_replace) &&
> stmt->space->run_triggers && (stmt->old_tuple || stmt->new_tuple)) {
> + /* Triggers see old_tuple and that tuple must remain the same */
> + stmt->does_require_old_tuple = true;
2. What about before_replace triggers?
> int rc = 0;
> if(!space_is_temporary(stmt->space)) {
> rc = trigger_run(&stmt->space->on_replace, txn);
> diff --git a/src/box/txn.h b/src/box/txn.h
> index e261852..962ada0 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 replaces old_tuple before current one and the flag
> + * is set - the current transaction will be aborted.
3. Do you mean the other transaction *commits* replacement of the old_tuple?
Because if there was another transaction touched the old tuple, but the current
one managed to commit earlier, the current txn won't be aborted. The other txn
will be.
> + * 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;
More information about the Tarantool-patches
mailing list