memtx: fix reverse iterators gap tracking#7410
Merged
alyapunov merged 2 commits intotarantool:masterfrom Aug 4, 2022
Merged
Conversation
Korablev77
approved these changes
Jul 15, 2022
Contributor
diff --git a/src/box/memtx_tree.cc b/src/box/memtx_tree.cc
index 1ec614f6c..b2ca40322 100644
--- a/src/box/memtx_tree.cc
+++ b/src/box/memtx_tree.cc
@@ -397,8 +397,13 @@ tree_iterator_prev_raw_base(struct iterator *iterator, struct tuple **ret)
struct memtx_tree_data<USE_HINT> *check =
memtx_tree_iterator_get_elem(&index->tree, &it->tree_iterator);
if (check == NULL || !memtx_tree_data_is_equal(check, &it->current)) {
+ bool exact = false;
it->tree_iterator = memtx_tree_lower_bound_elem(&index->tree,
- it->current, NULL);
+ it->current, &exact);
+ struct memtx_tree_data<USE_HINT> *tmp = memtx_tree_iterator_get_elem
+ (&index->tree, &it->tree_iterator);
+ tree_iterator_set_current(it, tmp);
+ assert(exact);
}
memtx_tree_iterator_prev(&index->tree, &it->tree_iterator);
struct tuple *successor = it->current.tuple;
@@ -491,8 +496,13 @@ tree_iterator_prev_equal_raw_base(struct iterator *iterator, struct tuple **ret)
struct memtx_tree_data<USE_HINT> *check =
memtx_tree_iterator_get_elem(&index->tree, &it->tree_iterator);
if (check == NULL || !memtx_tree_data_is_equal(check, &it->current)) {
+ bool exact = false;
it->tree_iterator = memtx_tree_lower_bound_elem(&index->tree,
- it->current, NULL);
+ it->current, &exact);
+ struct memtx_tree_data<USE_HINT> *tmp = memtx_tree_iterator_get_elem
+ (&index->tree, &it->tree_iterator);
+ tree_iterator_set_current(it, tmp);
+ assert(exact);
}
memtx_tree_iterator_prev(&index->tree, &it->tree_iterator);
struct tuple *successor = it->current.tuple;
diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c
index 2b858944b..0564dda96 100644
--- a/src/box/memtx_tx.c
+++ b/src/box/memtx_tx.c
@@ -2847,6 +2847,7 @@ memtx_tx_track_gap_slow(struct txn *txn, struct space *space, struct index *inde
}
}
assert(index->dense_id < story->index_count);
+ assert(story->link[index->dense_id].in_index != NULL);
rlist_add(&story->link[index->dense_id].nearby_gaps,
&item->in_nearby_gaps);
} else {This patch fixes the problem too, but without iteration over tuple stories. |
All the 'base' TREE index iterator `next` methods (including `tree_iterator_start`) internally set the iterator's current tuple to the one found in the index satisfying the conditions: setting the iterator's current tuple to the clarified one is redundant and moreover gives a performance penalty each iteration because of the iterator check logic. Needed for tarantool#7409 NO_CHANGELOG=refactoring NO_DOC=refactoring NO_TEST=refactoring
3a0489f to
3d789af
Compare
In case of reverse iterators, due to index limitations, we need to clarify the successor tuple early: this implies that the successor's story is not always at the top of the history chain, whilst we need to add the gap item to the story currently present in index — fix this by reusing the iterators' check logic to set the current iterator's tuple (which is considered the successor) to a tuple in index. CLoses tarantool#7409 NO_DOC=bugfix
3d789af to
6ea662b
Compare
drewdzzz
approved these changes
Jul 28, 2022
Contributor
drewdzzz
left a comment
There was a problem hiding this comment.
OK, but PR comment is stale.
alyapunov
approved these changes
Aug 4, 2022
| memtx_tree_lower_bound_elem(&index->tree, iterator->current, | ||
| &exact); | ||
| if (exact) { | ||
| struct memtx_tree_data<USE_HINT> *successor = |
Contributor
There was a problem hiding this comment.
Nit: I would not call it successor here since it is current in the context.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In case of reverse iterators, due to index limitations, we need to clarify
the successor tuple early: this implies that the successor's story is not
always at the top of the history chain, whilst we need to add the gap item
to the story currently present in index — fix this by explicitly retrieving
the top story in the successor's history chain.
CLoses #7409
NO_DOC=bugfix