Skip to content

Commit

Permalink
Fix a bug in the batcher
Browse files Browse the repository at this point in the history
  • Loading branch information
andyleiserson committed Sep 27, 2024
1 parent c5c0ccf commit 966160f
Showing 1 changed file with 18 additions and 1 deletion.
19 changes: 18 additions & 1 deletion ipa-core/src/protocol/context/batcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl<'a, B> Batcher<'a, B> {
while self.batches.len() <= batch_offset {
let (validation_result, _) = watch::channel::<bool>(false);
let state = BatchState {
batch: (self.batch_constructor)(self.first_batch + batch_offset),
batch: (self.batch_constructor)(self.first_batch + self.batches.len()),
validation_result,
pending_count: 0,
pending_records: bitvec![0; self.records_per_batch],
Expand Down Expand Up @@ -296,6 +296,23 @@ mod tests {
);
}

#[test]
fn makes_batches_out_of_order() {
// Regression test for a bug where, when adding batches i..j to fill in a gap in
// the batch deque prior to out-of-order requested batch j, the batcher passed
// batch index `j` to the constructor for all of them, as opposed to the correct
// sequence of indices i..=j.

let batcher = Batcher::new(1, 2, Box::new(std::convert::identity));
let mut batcher = batcher.lock().unwrap();

batcher.get_batch(RecordId::from(1));
batcher.get_batch(RecordId::from(0));

assert_eq!(batcher.get_batch(RecordId::from(0)).batch, 0);
assert_eq!(batcher.get_batch(RecordId::from(1)).batch, 1);
}

#[tokio::test]
async fn validates_batches() {
let batcher = Batcher::new(2, 4, Box::new(|_| Vec::new()));
Expand Down

0 comments on commit 966160f

Please sign in to comment.