[ENH] Handle one-off compaction message in compaction manager #3379
+76
−44
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Sicheng-Pan
force-pushed
the
12-30-_enh_implement_compactor_server_interface
branch
from
b4533be to
6adc66c
Compare
Sicheng-Pan
force-pushed
the
12-31-_enh_handle_one-off_compaction_message_in_compaction_manager
branch
from
30b27ce to
45b2fe6
Compare
Sicheng-Pan
force-pushed
the
12-30-_enh_implement_compactor_server_interface
branch
from
6adc66c to
f04bbfa
Compare
Sicheng-Pan
force-pushed
the
12-31-_enh_handle_one-off_compaction_message_in_compaction_manager
branch
from
45b2fe6 to
25ff5ed
Compare
1 task
Sicheng-Pan
force-pushed
the
12-30-_enh_implement_compactor_server_interface
branch
from
f04bbfa to
ccf3eb3
Compare
Sicheng-Pan
force-pushed
the
12-31-_enh_handle_one-off_compaction_message_in_compaction_manager
branch
2 times, most recently
from
fe94965 to
2e0c103
Compare
HammadB
reviewed
Jan 21, 2025
|
|
||
| tracing::info!("Running {} compaction jobs", job_futures.len()); | ||
|
|
||
| job_futures |
HammadB
reviewed
Jan 21, 2025
| &mut self, | ||
| compacted: &mut Vec<CollectionUuid>, | ||
| ) -> (u32, u32) { | ||
| pub(crate) async fn compact_batch(&mut self) -> Vec<CollectionUuid> { |
There was a problem hiding this comment.
Why did we change the interface of this away from num success/failed?
There was a problem hiding this comment.
The old interface seemed strange for me. We are able to tell the number of successful compactions using the length of the returned Vec. The number of successful/failed compactions are only examined in one test case, and not used anywhere else. I updated the interface so that it looks cleaner to me.
HammadB
approved these changes
Jan 21, 2025
Sicheng-Pan
force-pushed
the
12-30-_enh_implement_compactor_server_interface
branch
from
2245609 to
b9521bc
Compare
Sicheng-Pan
force-pushed
the
12-31-_enh_handle_one-off_compaction_message_in_compaction_manager
branch
from
2e0c103 to
d2d6721
Compare
Sicheng-Pan
changed the base branch from
12-30-_enh_implement_compactor_server_interface
to
graphite-base/3379
Sicheng-Pan
force-pushed
the
12-31-_enh_handle_one-off_compaction_message_in_compaction_manager
branch
from
d2d6721 to
e3fac40
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytestfor python,yarn testfor js,cargo testfor rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?