diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c index 946b53717020a9b39220ac654faaf5235bf48d60..76f5fa324c76c710d7d2f536c4e70196cee67741 100644 --- a/src/box/memtx_tx.c +++ b/src/box/memtx_tx.c @@ -307,18 +307,6 @@ point_hole_storage_key_equal(const struct point_hole_key *key, #define MH_SOURCE #include "salad/mhash.h" -/** - * Temporary (allocated on region) struct that stores a conflicting TX. - */ -struct memtx_tx_conflict { - /* The transaction that will conflict us upon commit. */ - struct txn *breaker; - /* The transaction that will conflicted by upon commit. */ - struct txn *victim; - /* Link in single-linked list. */ - struct memtx_tx_conflict *next; -}; - /** * Collect an allocation to memtx_tx_stats. */ @@ -422,10 +410,6 @@ memtx_tx_region_object_to_type(enum memtx_tx_alloc_object alloc_obj) { enum memtx_tx_alloc_type alloc_type = MEMTX_TX_ALLOC_TYPE_MAX; switch (alloc_obj) { - case MEMTX_TX_OBJECT_CONFLICT: - alloc_type = MEMTX_TX_ALLOC_CONFLICT; - break; - case MEMTX_TX_OBJECT_CONFLICT_TRACKER: case MEMTX_TX_OBJECT_READ_TRACKER: alloc_type = MEMTX_TX_ALLOC_TRACKER; @@ -450,10 +434,6 @@ memtx_tx_region_alloc_object(struct txn *txn, enum memtx_tx_alloc_type alloc_type = memtx_tx_region_object_to_type(alloc_obj); switch (alloc_obj) { - case MEMTX_TX_OBJECT_CONFLICT: - alloc = region_alloc_object(&txn->region, - struct memtx_tx_conflict, &size); - break; case MEMTX_TX_OBJECT_CONFLICT_TRACKER: alloc = region_alloc_object(&txn->region, struct tx_conflict_tracker, &size); @@ -1752,31 +1732,6 @@ memtx_tx_story_delete_is_visible(struct memtx_story *story, struct txn *txn, return false; } -/** - * Save @a breaker in list with head @a conflicts_head. New list node is - * allocated on a region of @a breaker. - * @return 0 on success, -1 on memory error. - */ -static int -memtx_tx_save_conflict(struct txn *breaker, struct txn *victim, - struct memtx_tx_conflict **conflicts_head) -{ - assert(breaker != victim); - struct memtx_tx_conflict *next_conflict; - next_conflict = memtx_tx_region_alloc_object(breaker, - MEMTX_TX_OBJECT_CONFLICT); - if (next_conflict == NULL) { - diag_set(OutOfMemory, sizeof(*next_conflict), "txn_region", - "txn conflict"); - return -1; - } - next_conflict->breaker = breaker; - next_conflict->victim = victim; - next_conflict->next = *conflicts_head; - *conflicts_head = next_conflict; - return 0; -} - /** * Scan a history starting with @a story in @a index for a @a visible_tuple. * If @a is_prepared_ok is true that prepared statements are visible for @@ -1828,7 +1783,7 @@ memtx_tx_check_dup(struct tuple *new_tuple, struct tuple *old_tuple, } else { diag_set(ClientError, errcode, space_name(space)); } - return -1; + return -1; } return 0; } @@ -1846,18 +1801,14 @@ point_hole_storage_find(struct index *index, struct tuple *tuple) } /** - * Check for possible conflicts during inserting @a new tuple, and given - * that it was real insertion, not the replacement of existing tuple. - * It's the moment where we can search for stored point hole trackers - * and detect conflicts. - * Since the insertions are not completed successfully, we better store - * conflicts to the special temporary storage @a collected_conflicts in - * other to become real conflict only when insertion success is inevitable. + * Check for possible conflict relations during insertion of @a new tuple, + * and given that it was a real insertion, not the replacement of existing + * tuple. It's the moment where we can search for stored point hole trackers + * and find conflict causes. */ static int check_hole(struct space *space, uint32_t index, - struct tuple *new_tuple, struct txn *inserter, - struct memtx_tx_conflict **collected_conflicts) + struct tuple *new_tuple, struct txn *inserter) { struct point_hole_item *list = point_hole_storage_find(space->index[index], new_tuple); @@ -1867,12 +1818,10 @@ check_hole(struct space *space, uint32_t index, struct point_hole_item *item = list; do { if (inserter != item->txn && - memtx_tx_save_conflict(inserter, item->txn, - collected_conflicts) != 0) + memtx_tx_cause_conflict(inserter, item->txn) != 0) return -1; item = rlist_entry(item->ring.next, struct point_hole_item, ring); - } while (item != list); return 0; } @@ -1890,104 +1839,30 @@ memtx_tx_track_read(struct txn *txn, struct space *space, struct tuple *tuple); /** * Check that replaced tuples in space's indexes does not violate common * replace rules. See memtx_space_replace_all_keys comment. - * (!) Version for the case when replaced tuple in the primary index is - * either NULL or clean. - * @return 0 on success or -1 on fail. - */ -static int -check_dup_clean(struct txn_stmt *stmt, struct tuple *new_tuple, - struct tuple **replaced, struct tuple **old_tuple, - enum dup_replace_mode mode, - struct memtx_tx_conflict **collected_conflicts) -{ - assert(replaced[0] == NULL || !replaced[0]->is_dirty); - struct space *space = stmt->space; - struct txn *txn = stmt->txn; - - if (memtx_tx_check_dup(new_tuple, *old_tuple, replaced[0], - mode, space->index[0], space) != 0) { - if (replaced[0] != NULL) - memtx_tx_track_read(txn, space, replaced[0]); - return -1; - } - - if (replaced[0] == NULL) - check_hole(space, 0, new_tuple, stmt->txn, - collected_conflicts); - - for (uint32_t i = 1; i < space->index_count; i++) { - /* - * Check that visible tuple is NULL or the same as in the - * primary index, namely replaced[0]. - */ - if (replaced[i] == NULL) { - /* NULL is OK. */ - check_hole(space, i, new_tuple, stmt->txn, - collected_conflicts); - continue; - } - if (!replaced[i]->is_dirty) { - /* Check like there's no mvcc. */ - if (memtx_tx_check_dup(new_tuple, replaced[0], - replaced[i], DUP_INSERT, - space->index[i], space) != 0) { - memtx_tx_track_read(txn, space, replaced[i]); - return -1; - } - continue; - } - - /* - * The replaced tuple is dirty. A chain of changes cannot lead - * to clean tuple, but in can lead to NULL, that's the only - * chance to be OK. - */ - struct memtx_story *second_story = memtx_tx_story_get(replaced[i]); - struct tuple *check_visible; - bool unused; - memtx_tx_story_find_visible_tuple(second_story, txn, i, - true, &check_visible, - &unused); - - if (memtx_tx_check_dup(new_tuple, replaced[0], check_visible, - DUP_INSERT, space->index[i], space) != 0) { - memtx_tx_track_read(txn, space, check_visible); - return -1; - } - - if (check_visible == NULL) - check_hole(space, i, new_tuple, stmt->txn, - collected_conflicts); - } - - *old_tuple = replaced[0]; - return 0; -} - -/** - * Check that replaced tuples in space's indexes does not violate common - * replace rules. See memtx_space_replace_all_keys comment. - * (!) Version for the case when replaced tuple is dirty. * @return 0 on success or -1 on fail. * * `is_own_change` is set to true iff `old_tuple` was modified (either * added or deleted) by `stmt`'s transaction. */ static int -check_dup_dirty(struct txn_stmt *stmt, struct tuple *new_tuple, - struct tuple **replaced, struct tuple **old_tuple, - enum dup_replace_mode mode, - struct memtx_tx_conflict **collected_conflicts, - bool *is_own_change) +check_dup(struct txn_stmt *stmt, struct tuple *new_tuple, + struct tuple **directly_replaced, struct tuple **old_tuple, + enum dup_replace_mode mode, bool *is_own_change) { - assert(replaced[0] != NULL && replaced[0]->is_dirty); struct space *space = stmt->space; struct txn *txn = stmt->txn; - struct memtx_story *old_story = memtx_tx_story_get(replaced[0]); struct tuple *visible_replaced; - memtx_tx_story_find_visible_tuple(old_story, txn, 0, true, - &visible_replaced, is_own_change); + if (directly_replaced[0] == NULL || !directly_replaced[0]->is_dirty) { + *is_own_change = false; + visible_replaced = directly_replaced[0]; + } else { + struct memtx_story *story = + memtx_tx_story_get(directly_replaced[0]); + memtx_tx_story_find_visible_tuple(story, txn, 0, true, + &visible_replaced, + is_own_change); + } if (memtx_tx_check_dup(new_tuple, *old_tuple, visible_replaced, mode, space->index[0], space) != 0) { @@ -1995,83 +1870,43 @@ check_dup_dirty(struct txn_stmt *stmt, struct tuple *new_tuple, return -1; } - if (visible_replaced == NULL) - check_hole(space, 0, new_tuple, stmt->txn, - collected_conflicts); - for (uint32_t i = 1; i < space->index_count; i++) { /* * Check that visible tuple is NULL or the same as in the - * primary index, namely visible_replaced. + * primary index, namely replaced[0]. */ - if (replaced[i] == NULL) { - /* NULL is OK. */ - check_hole(space, i, new_tuple, stmt->txn, - collected_conflicts); - continue; - } - if (!replaced[i]->is_dirty) { + if (directly_replaced[i] == NULL) + continue; /* NULL is OK in any case. */ + + struct tuple *visible; + if (!directly_replaced[i]->is_dirty) { + visible = directly_replaced[i]; + } else { /* - * Non-null clean tuple cannot be NULL or - * visible_replaced since visible_replaced is dirty. + * The replaced tuple is dirty. A chain of changes + * cannot lead to clean tuple, but in can lead to NULL, + * that's the only chance to be OK. */ - diag_set(ClientError, ER_TUPLE_FOUND, - space->index[i]->def->name, - space_name(space), - tuple_str(replaced[i]), - tuple_str(new_tuple)); - return -1; + struct memtx_story *story = + memtx_tx_story_get(directly_replaced[i]); + bool unused; + memtx_tx_story_find_visible_tuple(story, txn, + i, true, &visible, + &unused); } - struct memtx_story *second_story = memtx_tx_story_get(replaced[i]); - struct tuple *check_visible; - bool unused; - memtx_tx_story_find_visible_tuple(second_story, txn, i, true, - &check_visible, &unused); - - if (memtx_tx_check_dup(new_tuple, visible_replaced, - check_visible, DUP_INSERT, - space->index[i], space) != 0) { - memtx_tx_track_read(txn, space, visible_replaced); + if (memtx_tx_check_dup(new_tuple, visible_replaced, visible, + DUP_INSERT, space->index[i], + space) != 0) { + memtx_tx_track_read(txn, space, visible); return -1; } - - if (check_visible == NULL) - check_hole(space, i, new_tuple, stmt->txn, - collected_conflicts); } *old_tuple = visible_replaced; return 0; } -/** - * Check that replaced tuples in space's indexes does not violate common - * replace rules. See memtx_space_replace_all_keys comment. - * Call check_dup_clean or check_dup_dirty depending on situation. - * @return 0 on success or -1 on fail. - * - * `is_own_change` is set to true iff `old_tuple` was modified (either - * added or deleted) by `stmt`'s transaction. - */ -static int -check_dup_common(struct txn_stmt *stmt, struct tuple *new_tuple, - struct tuple **directly_replaced, struct tuple **old_tuple, - enum dup_replace_mode mode, - struct memtx_tx_conflict **collected_conflicts, - bool *is_own_change) -{ - struct tuple *replaced = directly_replaced[0]; - if (replaced == NULL || !replaced->is_dirty) - return check_dup_clean(stmt, new_tuple, directly_replaced, - old_tuple, mode, - collected_conflicts); - else - return check_dup_dirty(stmt, new_tuple, directly_replaced, - old_tuple, mode, - collected_conflicts, is_own_change); -} - static struct gap_item * memtx_tx_gap_item_new(struct txn *txn, enum iterator_type type, const char *key, uint32_t part_count); @@ -2196,12 +2031,6 @@ memtx_tx_history_add_insert_stmt(struct txn_stmt *stmt, struct memtx_story *add_story = NULL; struct memtx_story *created_story = NULL, *replaced_story = NULL; - /* - * List of transactions that will conflict us once one of them - * become committed. - */ - struct memtx_tx_conflict *collected_conflicts = NULL; - struct tuple *directly_replaced[space->index_count]; struct tuple *direct_successor[space->index_count]; uint32_t directly_replaced_count = 0; @@ -2223,9 +2052,8 @@ memtx_tx_history_add_insert_stmt(struct txn_stmt *stmt, /* Check overwritten tuple */ bool is_own_change = false; - int rc = check_dup_common(stmt, new_tuple, directly_replaced, - &old_tuple, mode, - &collected_conflicts, &is_own_change); + int rc = check_dup(stmt, new_tuple, directly_replaced, + &old_tuple, mode, &is_own_change); if (rc != 0) goto fail; @@ -2260,12 +2088,10 @@ memtx_tx_history_add_insert_stmt(struct txn_stmt *stmt, goto fail; } - /* Purge found conflicts. */ - while (collected_conflicts != NULL) { - if (memtx_tx_cause_conflict(collected_conflicts->breaker, - collected_conflicts->victim) != 0) + /* Collect point phantom read conflicts. */ + for (uint32_t i = 0; i < space->index_count; i++) { + if (check_hole(space, i, new_tuple, stmt->txn) != 0) goto fail; - collected_conflicts = collected_conflicts->next; } if (replaced_story != NULL) diff --git a/src/box/memtx_tx.h b/src/box/memtx_tx.h index 6a1067f0acb4bacef6615d31508d7e7ad6d10ea3..2c2a0c0db0c7a037c59a4e1e704ba4d90077c356 100644 --- a/src/box/memtx_tx.h +++ b/src/box/memtx_tx.h @@ -60,7 +60,9 @@ extern const char *memtx_tx_alloc_type_strs[]; */ enum memtx_tx_alloc_object { /** - * Object of type struct memtx_tx_conflict. + * Deprecated object of type struct memtx_tx_conflict. + * Left for statistics compatibility. + * TODO: remove it considering monitoring module. */ MEMTX_TX_OBJECT_CONFLICT = 0, /** diff --git a/test/box-luatest/gh_6150_memtx_tx_memory_monitoring_test.lua b/test/box-luatest/gh_6150_memtx_tx_memory_monitoring_test.lua index 9822f010e38a81232409cd8cee1bdb00444fdd77..a5220e17d3e011fe1907996966cdf21e724a8c2e 100644 --- a/test/box-luatest/gh_6150_memtx_tx_memory_monitoring_test.lua +++ b/test/box-luatest/gh_6150_memtx_tx_memory_monitoring_test.lua @@ -11,7 +11,6 @@ local SIZE_OF_STORY = 144 local SIZE_OF_TUPLE = 9 -- Size of xrow for tuples with 2 number fields local SIZE_OF_XROW = 147 -local SIZE_OF_CONFLICT = 24 -- Tracker can allocate additional memory, be careful! local SIZE_OF_READ_TRACKER = 56 local SIZE_OF_CONFLICT_TRACKER = 48 @@ -386,9 +385,9 @@ g.test_conflict = function() ["total"] = trackers_used, }, ["conflicts"] = { - ["max"] = SIZE_OF_CONFLICT, - ["avg"] = math.floor(SIZE_OF_CONFLICT / 2), - ["total"] = SIZE_OF_CONFLICT, + ["max"] = 0, + ["avg"] = 0, + ["total"] = 0, }, ["tuples"] = { ["used"] = {