memtx: fix story delete statement list#7215
Merged
alyapunov merged 1 commit intotarantool:masterfrom Jul 6, 2022
Merged
Conversation
5965bd4 to
3f115ab
Compare
3f115ab to
bc9fc39
Compare
alyapunov
reviewed
Jun 23, 2022
src/box/memtx_tx.c
Outdated
| * If @a is_prepared_ok is true that prepared statements are visible for | ||
| * that lookup, and not visible otherwise. | ||
| * | ||
| * `is_own_change` is set to true iff `old_tuple` was modified (either |
Contributor
There was a problem hiding this comment.
There's no old_tuple here
Member
Author
There was a problem hiding this comment.
Indeed, the comment was CP. Fixed: changed the comment to match the function signature.
alyapunov
reviewed
Jun 23, 2022
| struct point_hole_item *item = list; | ||
| do { | ||
| if (memtx_tx_save_conflict(inserter, item->txn, | ||
| if (inserter != item->txn && |
Member
Author
There was a problem hiding this comment.
Fixed: rebased on master.
507609e to
06afeff
Compare
06afeff to
9be6d21
Compare
Korablev77
approved these changes
Jul 4, 2022
| end) | ||
| end | ||
|
|
||
| g.test_repeatable_insert_primary_idx_gh_7217 = function() |
Contributor
There was a problem hiding this comment.
Split tests into two files with corresponding names.
Member
Author
There was a problem hiding this comment.
Fixed: split tests and changelogs into two files with names corresponding to GitHub tickets.
alyapunov
approved these changes
Jul 4, 2022
26d864b to
9d6a3e1
Compare
Current implementation of tracking statements that delete a story has a
flaw, consider the following example:
tx1('box.space.s:replace{0, 0}') -- statement 1
tx2('box.space.s:replace{0, 1}') -- statement 2
tx2('box.space.s:delete{0}') -- statement 3
tx2('box.space.s:replace{0, 2}') -- statement 4
When statement 1 is prepared, both statements 2 and 4 will be linked to the
delete statement list of {0, 0}'s story, though, apparently, statement 4
does not delete {0, 0}.
Let us notice the following: statement 4 is "pure" in the sense that, in
the transaction's scope, it is guaranteed not to replace any tuple — we
can retrieve this information when we check where the insert statement
violates replacement rules, use it to determine "pure" insert statements,
and skip them later on when, during preparation of insert statements, we
handle other insert statements which assume they do not replace anything
(i.e., have no visible old tuple).
On the contrary, statements 1 and 2 are "dirty": they assume that they
replaced nothing (i.e., there was no visible tuple in the index) — when one
of them gets prepared — the other one needs to be either aborted or
relinked to replace the prepared tuple.
We also need to fix relinking of delete statements from the older story
(in terms of the history chain) to the new one during preparation of insert
statements: a statement needs to be relinked iff it comes from a different
transaction (to be precise, there must, actually, be no more than one
delete statement from the same transaction).
Additionally, add assertions to verify the invariant that the story's
add (delete) psn is equal to the psn of the add (delete) statement's
transaction psn.
Closes tarantool#7214
Closes tarantool#7217
NO_DOC=bugfix
9d6a3e1 to
cd2bd28
Compare
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.
Current implementation of tracking statements that delete a story has a
flaw, consider the following example:
tx1('box.space.s:replace{0, 0}') -- statement 1
tx2('box.space.s:replace{0, 1}') -- statement 2
tx2('box.space.s:delete{0}') -- statement 3
tx2('box.space.s:replace{0, 2}') -- statement 4
When statment 1 is prepared, both statements 2 and 4 will be linked to the
delete statement list of {0, 0}'s story, though, apparently, statement 4
does not delete {0, 0}.
Let us notice the following: statement 4 is "pure" in the sense that, in
the transaction's scope, it is guaranteed not to replace any tuple — we can
retrieve this information when we check where the insert statement violates
replacement rules, use it to determine "pure" insert statements, and skip
them later on when, during preparation of insert statements, we handle
other insert statements which assume they do not replace anything (i.e.,
have no visible old tuple).
We also need to fix relinking of delete statements from the older story
(in terms of the history chain) to the new one during preparation of insert
statements: a statement needs to be relinked iff it comes from a different
transaction (to be precise, there must, actually, be no more than one
delete statement from the same transaction).
Additionally, added assertions to verify the invariant that the story's
add (delete) psn is equal to the psn of the add (delete) statement's
transaction psn and to verify that no self-conflicting of transactions
occurs.
Closes #7214
Closes #7217