Skip to content

Commit

Permalink
memtx: remove does_require_old_tuple member
Browse files Browse the repository at this point in the history
It was an ugly solution when MVCC engine requires outside engine
to set this flag which is not convenient.

Remove it and use mode arguments to set up proper read trackers.

Part of #8648
Part of #8654

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring

(cherry picked from commit 7414973)
  • Loading branch information
alyapunov committed Jun 23, 2023
1 parent 4981aaa commit f93a3da
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 52 deletions.
13 changes: 0 additions & 13 deletions src/box/memtx_space.c
Expand Up @@ -395,9 +395,6 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,
return -1;
tuple_ref(new_tuple);

if (mode == DUP_INSERT)
stmt->does_require_old_tuple = true;

if (memtx_space_replace_tuple(space, stmt, NULL, new_tuple,
mode) != 0) {
tuple_unref(new_tuple);
Expand Down Expand Up @@ -429,12 +426,6 @@ memtx_space_execute_delete(struct space *space, struct txn *txn,
return 0;
}

/*
* We have to delete exactly old_tuple just because we return it as
* a result.
*/
stmt->does_require_old_tuple = true;

if (memtx_space_replace_tuple(space, stmt, old_tuple, NULL,
DUP_REPLACE_OR_INSERT) != 0)
return -1;
Expand Down Expand Up @@ -490,8 +481,6 @@ memtx_space_execute_update(struct space *space, struct txn *txn,
return -1;
tuple_ref(new_tuple);

stmt->does_require_old_tuple = true;

if (memtx_space_replace_tuple(space, stmt, old_tuple, new_tuple,
DUP_REPLACE) != 0) {
tuple_unref(new_tuple);
Expand Down Expand Up @@ -611,8 +600,6 @@ memtx_space_execute_upsert(struct space *space, struct txn *txn,
}
assert(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
Expand Down
37 changes: 25 additions & 12 deletions src/box/memtx_tx.c
Expand Up @@ -2025,10 +2025,9 @@ memtx_tx_history_add_insert_stmt(struct txn_stmt *stmt,
memtx_tx_story_link_top(add_story, secondary_replaced, i, true);
}

struct memtx_story *del_story = NULL;
if (old_tuple != NULL) {
assert(old_tuple->is_dirty);

struct memtx_story *del_story = NULL;
if (old_tuple == replaced)
del_story = replaced_story;
else
Expand All @@ -2037,6 +2036,30 @@ memtx_tx_history_add_insert_stmt(struct txn_stmt *stmt,
} else if (is_own_change)
stmt->is_pure_insert = true;

/*
* In case of DUP_INSERT there must be no visible replaced tuple. It is
* correct by now (checked in check_dup), but we must prevent further
* insertion to this place, so we have to track gap.
* In case of replace we usually does not depend on presence of absence
* of old tuple, but if there is a trigger - it takes old_tuple (NULL or
* non-NULL) as a side effect, so we must track it to remain the same.
* Note that none of the above is needed the previous action in this
* point of in index is made by the same transaction. For example, if
* a transaction replaces, deletes and then inserts some key - no other
* transaction can interfere with insert: due to serialization the
* previous delete statement guarantees that the insert will not fail.
*/
if (!is_own_change &&
(mode == DUP_INSERT ||
!rlist_empty(&stmt->space->before_replace) ||
!rlist_empty(&stmt->space->on_replace))) {
assert(mode != DUP_INSERT || del_story == NULL);
if (del_story == NULL)
memtx_tx_track_story_gap(stmt->txn, add_story, 0);
else
memtx_tx_track_read_story(stmt->txn, space, del_story);
}

*result = old_tuple;
if (*result != NULL) {
/*
Expand Down Expand Up @@ -2307,9 +2330,6 @@ memtx_tx_history_prepare_insert_stmt(struct txn_stmt *stmt)
== test_stmt->txn);
continue;
}
if (test_stmt->does_require_old_tuple)
memtx_tx_handle_conflict(stmt->txn,
test_stmt->txn);

memtx_tx_story_link_deleted_by(story, test_stmt);
}
Expand Down Expand Up @@ -2363,10 +2383,6 @@ memtx_tx_history_prepare_insert_stmt(struct txn_stmt *stmt)
test_stmt->next_in_del_list = NULL;
test_stmt->del_story = NULL;

if (test_stmt->does_require_old_tuple)
memtx_tx_handle_conflict(stmt->txn,
test_stmt->txn);

/* Link to story's list. */
test_stmt->del_story = story;
*to = test_stmt;
Expand Down Expand Up @@ -2468,9 +2484,6 @@ memtx_tx_history_prepare_delete_stmt(struct txn_stmt *stmt)
*itr = test_stmt->next_in_del_list;
test_stmt->next_in_del_list = NULL;
test_stmt->del_story = NULL;
/* Conflict only in case of dependance. */
if (test_stmt->does_require_old_tuple)
memtx_tx_handle_conflict(stmt->txn, test_stmt->txn);
}

struct tx_read_tracker *tracker;
Expand Down
14 changes: 0 additions & 14 deletions src/box/txn.c
Expand Up @@ -311,7 +311,6 @@ txn_stmt_new(struct txn *txn)
stmt->engine_savepoint = NULL;
stmt->row = NULL;
stmt->has_triggers = false;
stmt->does_require_old_tuple = false;
stmt->is_pure_insert = false;
return stmt;
}
Expand Down Expand Up @@ -636,20 +635,7 @@ txn_commit_stmt(struct txn *txn, struct request *request)
*/
if (stmt->space != NULL && stmt->space->run_triggers &&
(stmt->old_tuple || stmt->new_tuple)) {
if (!rlist_empty(&stmt->space->before_replace)) {
/*
* Triggers see old_tuple and that tuple
* must remain the same
*/
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;

txn->space_on_replace_triggers_depth++;
int rc = trigger_run(&stmt->space->on_replace, txn);
txn->space_on_replace_triggers_depth--;
Expand Down
13 changes: 0 additions & 13 deletions src/box/txn.h
Expand Up @@ -298,19 +298,6 @@ 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;
/*
* `insert` statement is guaranteed not to delete anything
* from the transaction's point of view (i.e., there was a preceding
Expand Down

0 comments on commit f93a3da

Please sign in to comment.