-
Notifications
You must be signed in to change notification settings - Fork 204
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: support pick ssts in compact #1603
Conversation
for key in all_keys.into_iter().rev() { | ||
if let Some(bucket) = buckets.get(key) { | ||
if bucket.len() >= 2 { | ||
compact_files = bucket.clone(); |
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.
We need to limit input compact files, also we should compact smaller files first.
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.
We a SST is picked, we should mark it in compacting, so it won't got compacted twice.
horaedb/metric_engine/src/sst.rs
Outdated
pub fn is_uncompacted(&self) -> bool { | ||
!self.inner.in_compaction.load(Ordering::Relaxed) | ||
} |
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.
pub fn is_uncompacted(&self) -> bool { | |
!self.inner.in_compaction.load(Ordering::Relaxed) | |
} | |
pub fn is_compaction(&self) -> bool { | |
self.inner.in_compaction.load(Ordering::Relaxed) | |
} |
Negate this bool inside this function is not very readable.
|
||
let bucket_files = buckets.entry(left).or_default(); | ||
|
||
bucket_files.push(f.id()); |
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.
FileId is not suitable here, we need to mutate sst inner state, so a Task should hold SstFile
buckets | ||
} | ||
|
||
fn get_window_bounds(&self, ts: Timestamp) -> (i64, i64) { |
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 UT for this function.
f579c45
to
ef04238
Compare
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
Rationale
Implement pick_candidate for compaction
Detailed Changes
Test Plan
UT