Skip to content

memtx: fix a crash caused by mhash misusage in MVCC#11162

Merged
sergepetrenko merged 1 commit intotarantool:masterfrom
drewdzzz:mvcc_mhash_misusage
Feb 18, 2025
Merged

memtx: fix a crash caused by mhash misusage in MVCC#11162
sergepetrenko merged 1 commit intotarantool:masterfrom
drewdzzz:mvcc_mhash_misusage

Conversation

@drewdzzz
Copy link
Contributor

We use mhash in memtx MVCC to store trackers of reads that have read nothing, we call them point holes. When handling a write to such hole, the tracker should be deleted because we can use the new tuple to store reads instead. Deletion flow in mhash is: we find bucket of the element with find, then we free the bucket with del. It can seem that the element is not needed on del because bucket id is used. However, it can be used on incremental resize of mhash. And, since we delete the point holes before releasing the bucket in mhash, in the rare case of incremental resize Tarantool will be aborted by mhash consistency check. The commit fixes the problem - simply release the bucket before deleting the object.

Along the way, the commit fixes another misusage that doesn't acutally break something. Method put_slot is internal so shouldn't be used manually - let's use more convenient get instead.

Closes #11022

@coveralls
Copy link

coveralls commented Feb 18, 2025

Coverage Status

coverage: 87.416% (+0.01%) from 87.406%
when pulling 7a0aa92 on drewdzzz:mvcc_mhash_misusage
into d31bd73
on tarantool:master
.

@drewdzzz drewdzzz marked this pull request as ready for review February 18, 2025 10:21
@drewdzzz drewdzzz requested a review from a team as a code owner February 18, 2025 10:21
@mkostoevr mkostoevr assigned drewdzzz and unassigned mkostoevr Feb 18, 2025
We use mhash in memtx MVCC to store trackers of reads that have read
nothing, we call them point holes. When handling a write to such hole,
the tracker should be deleted because we can use the new tuple to store
reads instead. Deletion flow in mhash is: we find bucket of the element
with `find`, then we free the bucket with `del`. It can seem that the
element is not needed on `del` because bucket id is used. However, it
can be used on incremental resize of mhash. And, since we delete the
point holes before releasing the bucket in mhash, in the rare case of
incremental resize Tarantool will be aborted by mhash consistency check.
The commit fixes the problem - simply release the bucket before deleting
the object.

Along the way, the commit fixes another misusage that doesn't actually
break something. Method `put_slot` is internal so shouldn't be used
manually - let's use more convenient `get` instead.

Closes #11022

NO_DOC=bugfix
@drewdzzz drewdzzz added full-ci Enables all tests for a pull request backport/2.11 Automatically create a 2.11 backport PR backport/3.2 Automatically create a 3.2 backport PR backport/3.3 Automatically create a 3.3 backport PR and removed full-ci Enables all tests for a pull request labels Feb 18, 2025
@drewdzzz drewdzzz assigned sergepetrenko and unassigned drewdzzz Feb 18, 2025
@sergepetrenko sergepetrenko merged commit 7c73e24 into tarantool:master Feb 18, 2025
99 of 175 checks passed
@TarantoolBot
Copy link
Collaborator

Successfully created backport PR for release/2.11:

@TarantoolBot
Copy link
Collaborator

Successfully created backport PR for release/3.2:

@TarantoolBot
Copy link
Collaborator

Successfully created backport PR for release/3.3:

@TarantoolBot
Copy link
Collaborator

Backport summary

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

Labels

backport/2.11 Automatically create a 2.11 backport PR backport/3.2 Automatically create a 3.2 backport PR backport/3.3 Automatically create a 3.3 backport PR full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Abort has triggered in mh_point_holes_resize

7 participants