google / google/capsem

[triage:tooling-03-dns-bind-failure-sample-count-mismatch] DNS worker bind failure collapses `count` requests into one sample, corrupting `failed`/`successful`/throughput

Open
#147 0 comments 0 reactions 0 assignees View on GitHub
type:bug
Dominant language
Rust
Stars
72
Forks
13
Avg merge
1d 2h
Merged PRs (30d)
5

Description

Imported from Capsem triage report `tooling-03-dns-bind-failure-sample-count-mismatch.md`.

- Severity: `low`
- Category: `bug (measurement correctness)`
- Area: `capsem-bench`
- Location: `crates/capsem-bench/src/main.rs:624-637` (DNS worker bind-error path) and `summarize()` at `798-853`
- Confidence: `verified`

## Summary
When a DNS worker fails to bind its UDP socket, it returns a single error `RequestSample` instead of the `count` samples it was assigned. `summarize()` then derives `failed = total_requests - successful` from the *configured* request count, not from the number of samples actually taken. The result is an internally inconsistent report: the `errors` map records one `dns bind:` error, but `failed` is inflated to the whole missing batch, and `requests_per_sec` is computed against `total_requests` that were never sent.

## Evidence
Worker on bind failure (lines 625-636):
```rust
let socket = match UdpSocket::bind("0.0.0.0:0").await {
Ok(socket) => socket,
Err(error) => {
return vec![RequestSample { status: 0, size: 0, latency_ms: 0.0,
error: Some(format!("dns bind: {error}")), ... }]; // ONE sample, not `count`
}
};
```
`summarize()` (lines 805-845):
```rust
let successful = samples.iter().filter(|s| result_ok(s, scenario)).count();
... // errors map increments once per sample
failed: total_requests.saturating_sub(successful),
requests_per_sec: round1(total_requests as f64 / duration_s),
```
So with N total requests across W workers, if one worker's bind fails: `samples.len() == total_requests - count_of_that_worker + 1`. `successful` ≈ `total_requests - count`. `failed` is reported as `count`, but the `errors` map sums to `1`. `requests_per_sec`/`bytes_per_sec` still divide by `total_requests`.

`validate_successful_scenarios` (line 924) keys on `row.failed > 0`, so it would correctly abort — but the emitted artifact (already written by `write_json` before any caller inspects it in the non-delta path) carries numbers where `failed` and the `errors` map disagree.

## Impact
Low real-world frequency (UDP ephemeral-port bind rarely fails), but when it does the JSON artifact is self-contradictory: `failed=count` with a single `errors` entry, and throughput computed over unsent requests. Anyone diffing artifacts or summing the `errors` map to reconcile `failed` gets wrong totals.

## Suggested fix
On bind failure, push `count` identical error samples (loop `0..count`) so the sample count always equals the assigned work, keeping `failed`, the `errors` map, and throughput mutually consistent. Alternatively, compute `failed`/throughput from `samples.len()` rather than `total_requests`.

## Triage
Confirmed from the local reviewed report in `/Users/elie/git/capsem/tmp/bugs/tooling-03-dns-bind-failure-sample-count-mismatch.md`. Track implementation in the triage sprint; add regression coverage before fixing.

Contributor guide

Open the contributing guide

Research direction

Start in crates/capsem-bench/src/main.rs at the DNS worker bind-error path around lines 624-637, then trace summarize() around lines 798-853 and validate_successful_scenarios at line 924. Add regression coverage for a bind failure affecting an assigned count, and verify the emitted failed count, errors map, sample totals, and throughput remain consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
performance, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.