memtx: do not abort unrelated transactions on DDL#10833
Merged
locker merged 1 commit intotarantool:masterfrom Nov 28, 2024
drewdzzz:memtx_mvcc_ddl_abort_related
Merged
memtx: do not abort unrelated transactions on DDL#10833locker merged 1 commit intotarantool:masterfrom drewdzzz:memtx_mvcc_ddl_abort_related
locker merged 1 commit intotarantool:masterfrom
drewdzzz:memtx_mvcc_ddl_abort_related
Conversation
p7nov
reviewed
Nov 21, 2024
changelogs/unreleased/gh-10377-do-not-abort-unrelated-on-ddl.md
Outdated
Show resolved
Hide resolved
p7nov
approved these changes
Nov 21, 2024
CuriousGeorgiy
requested changes
Nov 21, 2024
Member
CuriousGeorgiy
left a comment
There was a problem hiding this comment.
Great that you found such a simple solution to this problem by reusing existing read/write sets! I'm loving it.
test/engine-luatest/gh_10375_ddl_does_not_abort_unrelated_transactions_test.lua
Outdated
Show resolved
Hide resolved
CuriousGeorgiy
approved these changes
Nov 22, 2024
locker
reviewed
Nov 25, 2024
Currently we simply abort all concurrent transactions on any DDL. We've already done a few workarounds for these problems - filter that prevents DDL on temporary spaces from aborting fully remote transactions and vice versa, and an engine flag `ENGINE_TXM_HANDLES_DDL` to prevent memtx DDL from aborting vinyl transactions. The commit removes above-mentioned temporary solutions and makes memtx abort only related transactions on DDL. The new approach is to abort all writers and readers when a space is invalidated (it happens on replace in space cache). To abort all writers, we can iterate over all transactions and inspect their statements. To abort all readers, MVCC trackers can be used. When MVCC is disabled, we don't abort any transactions since memtx ones cannot yield. Note that we swap indexes on DDL before the old space is invalidated, so all index read gaps are actually moved to the new space. It's actually incorrect but didn't break anything - we delete all transaction's trackers when it is prepared or aborted, and we used to abort all in-progress transactions on DDL. But with the new approach, we need those gaps in the old space in order to find all space readers, so the commit swaps read gaps back when moving space indexes. A primary memtx difference is that it contains all system spaces, and any schema change is a replace in a system space, hence, in memtx. So the question is whether we should abort any active transactions on schema changes that don't affect any space directly (for example, bump of schema version on upgrade, replace in space `_schema`). The answer is we shouldn't - all resources used by spaces are pinned, so we cannot change, for example, a collation when it is used by a space, so let's abort concurrent transactions only when a space is invalidated - it happens on DDL operations directly affecting spaces (`space:format(...)`, `space:create_index(...)` and so on). Note that despite the commit populates `point_hole_item` with a new member, its size remains the same - the object has a 8-byte alignment and because of the `bool` in the end 7 bytes are wasted, so the new member is put right to the end right before the boolean to use the space used for alignment. Closes #10377 NO_DOC=not documentable feature
locker
approved these changes
Nov 26, 2024
Contributor
Author
|
Decided to backport to 3.2 but not to 2.11. |
Member
|
Cherry-picked to 3.2. |
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
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.
See the commits for details.
Closes #10377