Skip to content

Commit 1cb848e

Browse files
committed
Migrate the load contract from positional to key-addressed
Positional Vec -> Vec output cannot detect reordering and misdelivers results silently. A key-addressed map makes violations detectable: a missing key is addressable (Error::MissingOutput), an unknown key fails the whole batch (Error::ContractViolation).
1 parent cb57f4a commit 1cb848e

3 files changed

Lines changed: 59 additions & 18 deletions

File tree

examples/quickstart.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
// Smoke demo: implement BatchCollector, read the default config, see that equal
2-
// inputs hash to one key. Run: cargo run --example quickstart
1+
// Smoke demo: implement BatchCollector, read the default config,
2+
// see that equal inputs map to the same key.
3+
// Run: cargo run --example quickstart
34
// Expected output:
45
// config: window=30ms max_batch=1024 timeout=30s concurrency=None max_waiting=None
5-
// key(7) = 7, key(7) = 7 -> one key, one shared ride
6+
// key(7) = 7, key(7) = 7 -> equal inputs share one batch entry
7+
8+
use std::collections::HashMap;
69

710
use carpool::{BatchCollector, BatchLoaderConfig};
811

@@ -18,8 +21,8 @@ impl BatchCollector for SquareLoader {
1821
*input
1922
}
2023

21-
async fn load(&self, inputs: Vec<u64>) -> Result<Vec<u64>, Self::Error> {
22-
Ok(inputs.iter().map(|n| n * n).collect())
24+
async fn load(&self, batch: HashMap<u64, u64>) -> Result<HashMap<u64, u64>, Self::Error> {
25+
Ok(batch.into_iter().map(|(k, n)| (k, n * n)).collect())
2326
}
2427
}
2528

@@ -36,5 +39,5 @@ fn main() {
3639

3740
let loader = SquareLoader;
3841
let (a, b) = (loader.key(&7), loader.key(&7));
39-
println!("key(7) = {a}, key(7) = {b} -> one key, one shared ride");
42+
println!("key(7) = {a}, key(7) = {b} -> equal inputs share one batch entry");
4043
}

src/collector.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::collections::HashMap;
12
use std::future::Future;
23
use std::hash::Hash;
34

@@ -9,11 +10,17 @@ pub trait BatchCollector: Send + Sync + 'static {
910

1011
fn key(&self, input: &Self::Input) -> Self::Key;
1112

12-
// inputs are deduplicated by key; output must be position-aligned to them
13+
// batch arrives deduplicated: inputs sharing a key are interchangeable,
14+
// only one representative reaches load.
15+
// strict contract: every requested key must get a value;
16+
// a missing key is Error::MissingOutput for its waiters,
17+
// an unknown key in the response is Error::ContractViolation
18+
// for the whole batch (it takes precedence when both occur).
19+
// absence semantics live in the implementor's Output type.
1320
fn load(
1421
&self,
15-
inputs: Vec<Self::Input>,
16-
) -> impl Future<Output = Result<Vec<Self::Output>, Self::Error>> + Send;
22+
batch: HashMap<Self::Key, Self::Input>,
23+
) -> impl Future<Output = Result<HashMap<Self::Key, Self::Output>, Self::Error>> + Send;
1724
}
1825

1926
#[cfg(test)]
@@ -32,21 +39,22 @@ mod tests {
3239
*input
3340
}
3441

35-
async fn load(&self, inputs: Vec<u64>) -> Result<Vec<u64>, Self::Error> {
36-
Ok(inputs.iter().map(|x| x * x).collect())
42+
async fn load(&self, batch: HashMap<u64, u64>) -> Result<HashMap<u64, u64>, Self::Error> {
43+
Ok(batch.into_iter().map(|(k, x)| (k, x * x)).collect())
3744
}
3845
}
3946

4047
// Spawning the load future proves it is `Send` (the whole point of the
4148
// RPITIT + Send form) and that a plain `async fn` satisfies the bound.
4249
#[tokio::test]
43-
async fn load_future_is_send_and_position_aligned() {
50+
async fn load_future_is_send_and_key_addressed() {
4451
let loader = SquareLoader;
45-
let out = tokio::spawn(async move { loader.load(vec![2, 3, 4]).await })
52+
let batch = HashMap::from([(2, 2), (3, 3), (4, 4)]);
53+
let out = tokio::spawn(async move { loader.load(batch).await })
4654
.await
4755
.expect("task joins")
4856
.expect("load succeeds");
4957

50-
assert_eq!(out, vec![4, 9, 16]);
58+
assert_eq!(out, HashMap::from([(2, 4), (3, 9), (4, 16)]));
5159
}
5260
}

src/error.rs

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ use std::fmt;
44
#[non_exhaustive]
55
pub enum Error<E> {
66
Collector(E),
7-
ContractViolation { expected: usize, got: usize },
7+
// implementor bug: the response carries keys that were not in the batch;
8+
// not attributable to any single caller, so the whole batch fails
9+
// TODO: consider #[non_exhaustive] on this variant before 1.0 -
10+
// enum-level #[non_exhaustive] does not stop exhaustive field destructuring
11+
ContractViolation { unknown_keys: usize },
12+
// implementor bug: no output under a requested key; not a domain "not found" -
13+
// absence semantics belong to the implementor's Output type
14+
MissingOutput,
815
Timeout,
916
WaitingTimeout,
1017
}
@@ -13,10 +20,11 @@ impl<E: fmt::Display> fmt::Display for Error<E> {
1320
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1421
match self {
1522
Error::Collector(e) => write!(f, "collector failed: {e}"),
16-
Error::ContractViolation { expected, got } => write!(
23+
Error::ContractViolation { unknown_keys } => write!(
1724
f,
18-
"collector broke the position-aligned contract: expected {expected} outputs, got {got}"
25+
"collector broke the key-addressed contract: {unknown_keys} unknown key(s) in the response"
1926
),
27+
Error::MissingOutput => f.write_str("collector returned no output for a requested key"),
2028
Error::Timeout => f.write_str("batch timed out"),
2129
Error::WaitingTimeout => f.write_str("timed out waiting for a concurrency slot"),
2230
}
@@ -27,7 +35,10 @@ impl<E: std::error::Error + 'static> std::error::Error for Error<E> {
2735
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
2836
match self {
2937
Error::Collector(e) => Some(e),
30-
Error::ContractViolation { .. } | Error::Timeout | Error::WaitingTimeout => None,
38+
Error::ContractViolation { .. }
39+
| Error::MissingOutput
40+
| Error::Timeout
41+
| Error::WaitingTimeout => None,
3142
}
3243
}
3344
}
@@ -54,4 +65,23 @@ mod tests {
5465

5566
assert_eq!(source.to_string(), "downstream: boom");
5667
}
68+
69+
// Contract-bug variants wrap nothing downstream:
70+
// no source, and their Display stands on its own without touching E.
71+
#[test]
72+
fn contract_bug_variants_have_no_source_and_render() {
73+
let missing: Error<DownstreamError> = Error::MissingOutput;
74+
let violation: Error<DownstreamError> = Error::ContractViolation { unknown_keys: 2 };
75+
76+
assert!(std::error::Error::source(&missing).is_none());
77+
assert!(std::error::Error::source(&violation).is_none());
78+
assert_eq!(
79+
missing.to_string(),
80+
"collector returned no output for a requested key"
81+
);
82+
assert_eq!(
83+
violation.to_string(),
84+
"collector broke the key-addressed contract: 2 unknown key(s) in the response"
85+
);
86+
}
5787
}

0 commit comments

Comments
 (0)