Skip to content

vinyl: fix cache read view#11148

Merged
locker merged 3 commits intotarantool:masterfrom
locker:vy-cache-read-view-fix
Feb 18, 2025
Merged

vinyl: fix cache read view#11148
locker merged 3 commits intotarantool:masterfrom
locker:vy-cache-read-view-fix

Conversation

@locker
Copy link
Member

@locker locker commented Feb 13, 2025

This commit fixes a few issues in the tuple cache when a transaction operating in a read view could skip tuples deleted after the read view was created.

Closes #11079

@nshy nshy assigned locker and unassigned nshy Feb 17, 2025
If a read iterator skips a DELETE statement from the current transaction
write set, it clears the last cached entry so as not to create a cache
chain jumping over the old key version, which is still visible by other
transactions. The problem is that it doesn't clear the is_first_cached
flag. As a result, if there happens to be a DELETE statement in
the transaction write set preceding the first returned tuple, a read
iterator will still create a so-called "boundary" cache chain. We must
clear the flag to avoid that.

Part of tarantool#11079

NO_DOC=bug fix
NO_CHANGELOG=added later
The tuple cache can store chains. If a read iterator sees a chain,
it may skip deeper sources assuming there's nothing there. The problem
with cache chains is that we don't store their LSNs anywhere, as
a result a read iterator operating in a read view may erroneously skip
tuples deleted after the read view was created. Let's fix this issue by
storing the LSNs of the left- and right-adjacent chains in each cache
node. The LSN is initialized by the read iterator as the max LSN among
all skipped DELETE statements lying in the chain.

Part of tarantool#11079

NO_DOC=bug fix
NO_CHANGELOG=added later
Each cache chain stores the max LSN among all DELETE statements that are
known to fall in the chain. This LSN is required so that an older read
view can ignore the chain, otherwise it could skip tuples deleted after
the read view was created (see the previous commit).

The problem is, apart from regular DELETE statements, there may be so
called "deferred" ones. Deferred DELETE statements aren't written to
secondary indexes immediately on commit - this is done later, when
the primary index is compacted. If we don't find the corresponding
full tuple in the primary index by a partial tuple key fetched from
a secondary index, we skip it and create a cache chain leaping over
it so that we don't read it again on the next scan. We don't take
into account the deferred DELETE LSN in this case, as a result,
we face the same problem we fixed in the previous commit for regular
DELETEs.

To avoid that, let's keep track of the max LSN among all skipped
statements and use it for the cache chain. Note, we have to patch
vy_point_lookup() so that it can keep a DELETE statement instead of
returning NULL - we need it to extract its LSN for the cache. This
changes the stats a bit hence the vinyl/deferred_delete test update.

Closes tarantool#11079

NO_DOC=bug fix
@locker locker force-pushed the vy-cache-read-view-fix branch from bb548e8 to e6d98cf Compare February 17, 2025 14:26
@locker locker added the full-ci Enables all tests for a pull request label Feb 17, 2025
@coveralls
Copy link

Coverage Status

coverage: 87.416%. remained the same
when pulling e6d98cf on locker:vy-cache-read-view-fix
into ceadd1e
on tarantool:master
.

@locker locker merged commit d31bd73 into tarantool:master Feb 18, 2025
95 checks passed
@locker locker deleted the vy-cache-read-view-fix branch February 18, 2025 07:42
@locker
Copy link
Member Author

locker commented Feb 18, 2025

Cherry-picked to 2.11, 3.2, 3.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vinyl read view iterator may skip tuple deleted later

4 participants