vinyl: fix duplicate multikey stmt accounting with deferred deletes#10807
Merged
locker merged 2 commits intotarantool:masterfrom Nov 12, 2024
Merged
Conversation
`vy_mem_insert()` and `vy_mem_insert_upsert()` increment the row count statistic of `vy_mem` only if no statement is replaced, which is correct, while `vy_lsm_commit()` increments the row count of `vy_lsm` unconditionally. As a result, `vy_lsm` may report a non-zero statement count (via `index.stat()` or `index.len()`) after a dump. This may happen only with a non-unique multikey index, when the statement has duplicates in the indexed array, and only if the `deferred_deletes` option is enabled, because otherwise we drop duplicates when we form the transaction write set, see `vy_tx_set()`. With `deferred_deletes`, we may create a `txv` for each multikey entry at the time when we prepare to commit the transaction, see `vy_tx_handle_deferred_delete()`. Another problem is that `vy_mem_rollback_stmt()` always decrements the row count, even if it didn't find the rolled back statement in the tree. As a result, if the transaction with duplicate multikey entries is rolled back on WAL error, we'll decrement the row count of `vy_mem` more times than necessary. To fix this issue, let's make the `vy_mem` methods update the in-memory statistic of `vy_lsm`. This way they should always stay in-sync. Also, we make `vy_mem_rollback_stmt()` skip updating the statistics in case the rolled back statement isn't present in the tree. This issue results in `vinyl-luatest/select_consistency_test.lua` flakiness when checking `index.len()` after compaction. Let's make the test more thorough and also check that `index.len()` equals `index.count()`. Closes tarantool#10751 Part of tarantool#10752 NO_DOC=bug fix
The test expects at least 10 dumps to be created in 60 seconds. It usually works but sometimes, when the host is heavy loaded, Vinyl doesn't produce enough dumps in time and fails the test. On CI the test usually fails with 7-9 dumps. To avoid flaky failures, let's reduce the expected dump count down to 5. Closes tarantool#10752 NO_DOC=test fix NO_CHANGELOG=test fix
a66a8a9 to
6d6b1a0
Compare
p7nov
approved these changes
Nov 12, 2024
Member
Author
|
Cherry-picked to 2.11 and 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.
vinyl-luatest/select_consistency_test.luarevealed a problem with accounting of duplicate multikey statements in memory in case thedefer_deletesoption is enabled. This PR fixes the problem and also makes the test more thorough and robust.Closes #10751
Closes #10752