-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ENH] Add a safety cutoff to the rust log service. #4535
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Safety Cutoff Mechanism for Rust Log Service This PR adds a backpressure mechanism to prevent log service collections from accumulating too many uncompacted records. When a collection exceeds a configurable threshold (default 1,000,000 records), the system rejects further writes to that collection until compaction occurs. This protects the system from memory exhaustion when compaction falls behind. Key Changes: Affected Areas: This summary was automatically generated by @propel-code-bot |
@@ -434,6 +437,7 @@ impl DirtyMarker { | |||
.into_iter() | |||
.flat_map(Result::ok) | |||
.collect::<HashMap<_, _>>(); |
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.
[BestPractice]
Consider using explicit type parameters for HashMap collections. Use of the _
placeholder for type parameters reduces code clarity and can make it harder to understand the data structures being used.
.collect::<HashMap<_, _>>(); | |
.collect::<HashMap<CollectionUuid, (Arc<Storage>, Option<Manifest>, Option<Witness>)>>(); |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
rust/log-service/src/lib.rs
Outdated
use std::future::Future; | ||
use std::sync::Arc; | ||
use std::sync::{Arc, Mutex}; |
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 have adopted parking_lot
across our codebase.
If a collection on the rust-based log has more than a configuration-configurable number of records, shut it down hard no matter what. This happens when compaction gets backlogged. By default, no more than 1_000_000 can be on the log uncompacted. This is just a placeholder that we can change.
Description of changes
If a collection on the rust-based log has more than a
configuration-configurable number of records, shut it down hard no
matter what. This happens when compaction gets backlogged. By default,
no more than 1_000_000 can be on the log uncompacted. This is just a
placeholder that we can change.
Test plan
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
N/A