-
Notifications
You must be signed in to change notification settings - Fork 590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(storage): only update related read version on flush finish #16725
Conversation
IIRC we set the |
When spill happens, it's likely to trigger a shared buffer compaction that merge multiple imms into ssts. This serves similar functionality to merge imm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to deprecated the config related to "merge imm", Rest LGTM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Currently in event handler, when flush task finishes, we need to update all instances of read version, even though the flush task does not include the imm of the read version. In this PR, we change to store the mapping from instance id to its included imm in the output, so that we can know which instances should be updated and avoid updating all instances.
Besides, the algorithm to update each read version can be simplified. Previously, we need to do time-consuming sanity check when we update the read version. After this PR, since we separate the input imm of each read version shard, when we update the read version, we only need to check whether the input is at the imm queue end, which is greatly simplified.
The implementation of
for_each_read_version
is also modified. Previously, we will blocking wait for lock acquisition, even though we are able to acquire the lock of other read versions. In this PR, in the first round, we only calltry_write
on each read version. Thetry_write
succeeds, we do the work, and otherwise, we add the instance id to a queue. And then we will keep trying to acquire the write lock for instance at the queue front for at most 10 milli second, if it fails, we will move it to queue end and continue trying to acquire the write lock for the next item.The code of merge imm is removed by the way in this PR, because if we consider merged imm, we need to maintain the order of normal imm and merged imm, which unnecessarily increases the complexity.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.