vinyl: fix handling of overwritten statements in transaction write set
Statements executed in a transaction are first inserted into the transaction write set and only when the transaction is committed, they are applied to the LSM trees that store indexed keys in memory. If the same key is updated more than once in the same transaction, the old version is marked as overwritten in the write set and not applied on commit. Initially, write sets of different indexes of the same space were independent: when a transaction was applied, we didn't have a special check to skip a secondary index statement if the corresponding primary index statement was overwritten because in this case the secondary index statement would have to be overwritten as well. This changed when deferred DELETEs were introduced in commit a6edd455 ("vinyl: eliminate disk read on REPLACE/DELETE"). Because of deferred DELETEs, a REPLACE or DELETE overwriting a REPLACE in the primary index write set wouldn't generate DELETEs that would overwrite the previous key version in write sets of the secondary indexes. If we applied such a statement to the secondary indexes, it'd stay there forever because, since there's no corresponding REPLACE in the primary index, a DELETE wouldn't be generated on primary index compaction. So we added a special instruction to skip a secondary index statement if the corresponding primary index was overwritten, see `vy_tx_prepare()`. Actually, this wasn't completely correct because we skipped not only secondary index REPLACE but also DELETE. Consider the following example: ```lua local s = box.schema.space.create('test', {engine = 'vinyl'}) s:create_index('primary') s:create_index('secondary', {parts = {2, 'unsigned'}}) s:replace{1, 1} box.begin() s:update(1, {{'=', 2, 2}}) s:update(1, {{'=', 2, 3}}) box.commit() ``` UPDATEs don't defer DELETEs because, since they have to query the old value, they can generate DELETEs immediately so here's what we'd have in the transaction write set: 1. REPLACE {1, 2} in 'test.primary' [overwritten by no.4] 2. DELETE {1, 1} from 'test.secondary' 3. REPLACE {1, 2} in 'test.secondary' [overwritten by no.5] 4. REPLACE{1, 3} in 'test.primary' 5. DELETE{1, 2} from 'test.secondary' 6. REPLACE{1, 3} in 'test.secondary' Statement no.2 would be skipped and marked as overwritten because of the new check, resulting in {1, 1} never deleted from the secondary index. Note, the issue affects spaces both with and without enabled deferred DELETEs. This commit fixes this issue by updating the check to only skip REPLACE statements. It should be safe to apply DELETEs in any case. There's another closely related issue that affects only spaces with enabled deferred DELETEs. When we generate deferred DELETEs for secondary index when a transaction is committed (we can do it if we find the previous version in memory), we assume that there can't be a DELETE in a secondary index write set. This isn't true: there can be a DELETE generated by UPDATE or UPSERT. If there's a DELETE, we have nothing to do unless the DELETE was optimized out (marked as no-op). Both issues were found by `vinyl-luatest/select_consistency_test.lua`. Closes #10820 Closes #10822 NO_DOC=bug fix (cherry picked from commit 6a87c45deeb49e4e17ae2cc0eeb105cc9ee0f413)
Showing
- changelogs/unreleased/gh-10820-fix-vinyl-tx-stmt-overwrite.md 6 additions, 0 deletions...gelogs/unreleased/gh-10820-fix-vinyl-tx-stmt-overwrite.md
- src/box/vy_tx.c 38 additions, 26 deletionssrc/box/vy_tx.c
- test/vinyl-luatest/gh_10820_tx_stmt_overwrite_test.lua 166 additions, 0 deletionstest/vinyl-luatest/gh_10820_tx_stmt_overwrite_test.lua
Loading
Please register or sign in to comment