google / google/capsem

[triage:tooling-07-duplicated-worker-harness-http-dns] Worker/timing harness is duplicated between `run_http_scenario` and `run_dns_scenario`

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

Description

Imported from Capsem triage report `tooling-07-duplicated-worker-harness-http-dns.md`.

- Severity: `low`
- Category: `duplication`
- Area: `capsem-bench`
- Location: `crates/capsem-bench/src/main.rs:571-608` (HTTP) and `610-668` (DNS)
- Confidence: `verified`

## Summary
The two scenario runners share a byte-for-byte-identical work-splitting and timing skeleton: compute `workers`/`per_worker`/`remainder`, capture `started`, spawn one task per worker that loops `count` times, `try_join_all`, capture `wall_time`, flatten samples, and call `summarize`. Only the inner per-request call (and DNS's per-worker socket setup) differs. Any fix to the timing model — e.g. adding warmup or moving `Instant::now()` after spawn (see tooling-04) — must be applied in two places, and the DNS bind-failure inconsistency (tooling-03) exists precisely because the duplicated skeleton was copied without the per-request error-fanout that the HTTP path doesn't need.

## Evidence
HTTP (lines 580-607):
```rust
let workers = concurrency.min(total_requests);
let per_worker = total_requests / workers;
let remainder = total_requests % workers;
let started = Instant::now();
let tasks = (0..workers).map(|idx| { let count = per_worker + usize::from(idx < remainder);
tokio::spawn(async move { let mut out = Vec::with_capacity(count); for _ in 0..count { ... } out }) });
let joined = try_join_all(tasks).await...;
let wall_time = started.elapsed();
let samples = joined.into_iter().flatten().collect();
Ok(summarize(scenario, &samples, wall_time, total_requests, concurrency))
```
DNS (lines 618-667) is the same skeleton with `let count = per_worker + usize::from(idx < remainder);` and the identical `started`/`try_join_all`/`wall_time`/flatten/`summarize` tail.

## Impact
Two-place maintenance for any timing/warmup change; the duplication already produced a divergence (DNS bind-failure path emits one sample instead of `count`, tooling-03) that a shared harness would have structurally prevented.

## Suggested fix
Extract a generic `run_scenario(workers, total_requests, concurrency, scenario, |worker_idx, count| async { Vec })` that owns the split, timing, join, flatten, and `summarize` call. HTTP and DNS supply only the per-worker async closure. This also centralizes the fix point for warmup and post-spawn timing.

## Triage
Confirmed from the local reviewed report in `/Users/elie/git/capsem/tmp/bugs/tooling-07-duplicated-worker-harness-http-dns.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 run_http_scenario (lines 571-608) and run_dns_scenario (610-668), comparing their worker splitting, timing, joining, flattening, and summarize paths. Review the existing capsem-bench coverage before extracting the shared harness. Done means the duplicated skeleton is centralized, both scenarios retain their distinct per-request behavior, and regression coverage is added.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.