-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Refactor] Execute 'pick rowsets' before applying for permits for a compaction task #4891
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
[Refactor] Execute 'pick rowsets' before applying for permits for a compaction task #4891
Conversation
95a7742 to
8937a3b
Compare
| TRACE("got base compaction lock"); | ||
|
|
||
| if (_tablet->get_clone_occurred()) { | ||
| _tablet->set_clone_occurred(false); |
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.
Add comment to explain why setting clone occurred here.
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.
OK.
| BaseCompaction::~BaseCompaction() { } | ||
|
|
||
| OLAPStatus BaseCompaction::compact() { | ||
| RETURN_NOT_OK(prepare_compact()); |
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.
Who will call BaseCompaction::compact() now?
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.
Who will call
BaseCompaction::compact()now?
BaseCompaction::compact() will still be called in "be/src/http/action/compaction_action.cpp" for /api/compaction/run.
91b09c5 to
4f942cd
Compare
4f942cd to
c251ed0
Compare
morningman
left a comment
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.
LGTM
… to be cleared Regardless of whether the tablet is submitted for compaction or not, we need to call 'reset_compaction' to clean up the base_compaction or cumulative_compaction objects in the tablet, because these two objects store the tablet's own shared_ptr. If it is not cleaned up, the reference count of the tablet will always be greater than 1, thus cannot be collected by the garbage collector. (TabletManager::start_trash_sweep) This bug is introduced from apache#4891
… to be cleared (#5100) Regardless of whether the tablet is submitted for compaction or not, we need to call 'reset_compaction' to clean up the base_compaction or cumulative_compaction objects in the tablet, because these two objects store the tablet's own shared_ptr. If it is not cleaned up, the reference count of the tablet will always be greater than 1, thus cannot be collected by the garbage collector. (TabletManager::start_trash_sweep) This bug is introduced from #4891
… to be cleared (apache#5100) Regardless of whether the tablet is submitted for compaction or not, we need to call 'reset_compaction' to clean up the base_compaction or cumulative_compaction objects in the tablet, because these two objects store the tablet's own shared_ptr. If it is not cleaned up, the reference count of the tablet will always be greater than 1, thus cannot be collected by the garbage collector. (TabletManager::start_trash_sweep) This bug is introduced from apache#4891
Proposed changes
The current compaction mechanism is that there is a producer thread that has been producing compaction tasks, and the selected tablet must apply for
permits. When a tablet could holdpermits, compaction task for this tablet will be submitted to thread pool. We take compaction score aspermitswhich is used for limiting memory consumption. However,pick_rowset_to_compaction()will be executed before the file merge in compaction thread, and the number of segment files that actually perform the merge operation is smaller than compaction score. In addition, it is also possible that compaction task exits directly because the tablet doesn't meet the requirements of compaction.This patch optimizes and refactors the code of compaction, so that we can execute 'pick rowsets' before applying for permits for a compaction task, calculate the number of segment files that actually participate in the merge operation, and take this number as
permits.Types of changes
What types of changes does your code introduce to Doris?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.