-
Notifications
You must be signed in to change notification settings - Fork 2k
[ENH][wal3]: add quorum_writer for parallel future coordination #6095
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
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
The change introduces a Affected Areas• This summary was automatically generated by @propel-code-bot |
aacd96b to
1661c23
Compare
1661c23 to
a57a27d
Compare
26bc4bf to
adfd6a8
Compare
a57a27d to
af49672
Compare
adfd6a8 to
038ac2e
Compare
af49672 to
5ba021d
Compare
038ac2e to
ded3a45
Compare
5ba021d to
d1d3137
Compare
ded3a45 to
82b5763
Compare
d1d3137 to
ed229e0
Compare
82b5763 to
b90737d
Compare
ed229e0 to
2f00206
Compare
| let mut ok_count = 0; | ||
|
|
||
| // Phase 1: Wait for the minimum number of Ok futures to complete. | ||
| while ok_count < min_futures_to_wait_for { |
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.
So phase1 by design is without a deadline and can get stuck here forever? Would be useful to elaborate through comments here as to the intention
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.
The individual futures are assumed to be timeout-friendly, as is currently the case with all quorum ops. This gives the ability to control the timeout on a per-future basis without muddying this code.
b90737d to
4052be6
Compare
Implement a quorum-based coordination mechanism that: - Runs futures in parallel and waits for a minimum count of Ok results - Starts a timeout after reaching the quorum threshold - Cancels remaining futures that exceed the timeout - Returns results in original order with None for cancelled futures This enables handling partial quorum failures where some writers may be slow or unresponsive, allowing the system to proceed once a quorum of successful writes is achieved while still attempting to maximize replication within a bounded time window. Co-authored-by: AI
2f00206 to
06ee7ec
Compare
| type IndexedFuture<S, E> = Pin<Box<dyn Future<Output = (usize, Result<S, E>)> + Send>>; | ||
| let mut pending: FuturesUnordered<IndexedFuture<S, E>> = FuturesUnordered::new(); | ||
|
|
||
| for (idx, fut) in futures.into_iter().enumerate() { | ||
| pending.push(Box::pin(async move { (idx, fut.await) })); | ||
| } |
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.
[Performance] Since futures is a homogeneous Vec<F>, you can store the generated async blocks directly in FuturesUnordered without type erasure. This removes the need for Pin<Box<dyn Future...>>, saving one heap allocation per future and avoiding dynamic dispatch overhead.
The compiler will generate a single anonymous type for the async move block across all iterations.
Context for Agents
Since `futures` is a homogeneous `Vec<F>`, you can store the generated async blocks directly in `FuturesUnordered` without type erasure. This removes the need for `Pin<Box<dyn Future...>>`, saving one heap allocation per future and avoiding dynamic dispatch overhead.
The compiler will generate a single anonymous type for the `async move` block across all iterations.
File: rust/wal3/src/quorum_writer.rs
Line: 56
Description of changes
Implement a quorum-based coordination mechanism that:
This enables handling partial quorum failures where some writers may
be slow or unresponsive, allowing the system to proceed once a quorum
of successful writes is achieved while still attempting to maximize
replication within a bounded time window.
Test plan
Unit tests + CI
Migration plan
N/A
Observability plan
N/A
Documentation Changes
N/A
Co-authored-by: AI