Skip to content

Commit f3a9782

Browse files
committed
fix(gen): no rig limit may be indistinguishable from a gateway failure
Two of ours were, and one of them was a hole I left in this morning's fix. PORT EXHAUSTION was counted separately an hour ago and then still fell through to `fail`, so the window failed anyway and failed for our reason. Counting a thing apart is not the same as acting on it. A window containing connections THIS HOST could not make (EADDRNOTAVAIL/EMFILE) never ran at the concurrency it claims, which is precisely what `spawn_failed` already models, so it is unmeasured now - the same treatment, for the same reason. THE RESPONSE BUDGET is thirty seconds, and exhausting it returned the same `Exchange::Failed` a non-2xx does. It stays a failure, because a caller waiting thirty seconds for one request got nothing - but the bound is OURS, and the artifact could not say so. It is counted in both places now: in `fail` because it is one, and in `budget_exceeded` because a window failing for our timeout must not look identical to one failing for the gateway's. Both cross the subprocess boundary in the stats line, since that line is everything the parent learns from the child, and both parse leniently: an older generator's line simply has neither field, and absent means "not reported" rather than "did not happen". Refusing an otherwise complete line would turn a cosmetic version skew into a lost measurement. The budget is roughly a thousand times the slowest gateway this field has measured, so it should never fire. "Should never fire" is what was true of the ephemeral port range this morning, which is the whole argument for making it visible when it does. 405 tests pass. Confirmed RED with the counters stripped from the wire.
1 parent b31e961 commit f3a9782

3 files changed

Lines changed: 112 additions & 6 deletions

File tree

‎engine/src/gen.rs‎

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ pub struct GenStats {
7272
/// Counted separately so a window that ran out of rig can be recognised as one. EMFILE/ENFILE
7373
/// (out of file descriptors) are the same class and counted here too.
7474
pub rig_refused: u64,
75+
/// Requests that exhausted `RESPONSE_BUDGET`, a bound of OURS. Also counted in `fail`, because a
76+
/// caller waiting thirty seconds got nothing - but separable, so a window failing for our reason
77+
/// cannot look identical to one failing for the gateway's.
78+
pub budget_exceeded: u64,
7579
/// Every successful request's latency, microseconds. Percentiles are computed from this rather
7680
/// than from a running estimate: an approximate p99 is the one number nobody can check later.
7781
pub latencies_us: Vec<u64>,
@@ -125,7 +129,7 @@ impl GenStats {
125129
let p50 = Self::pct_of(&sorted, 0.50);
126130
let p99 = Self::pct_of(&sorted, 0.99);
127131
format!(
128-
"rps={} fail={} p50={:.2} p99={:.2} p50us={} p99us={} ok={} rigrefused={}",
132+
"rps={} fail={} p50={:.2} p99={:.2} p50us={} p99us={} ok={} rigrefused={} budgetexceeded={}",
129133
self.rps(),
130134
self.fail,
131135
p50 as f64 / 1000.0,
@@ -137,7 +141,8 @@ impl GenStats {
137141
// line is everything the parent learns from it, so a count that stops here would leave
138142
// the parent unable to tell its own port exhaustion from the gateway refusing - which is
139143
// the whole reason the count exists.
140-
self.rig_refused
144+
self.rig_refused,
145+
self.budget_exceeded
141146
)
142147
}
143148
}
@@ -152,13 +157,16 @@ struct WorkerStats {
152157
/// Connections this HOST could not make (ephemeral ports or descriptors exhausted), as opposed
153158
/// to requests the gateway refused. See `GenStats::rig_refused`.
154159
rig_refused: u64,
160+
/// Requests that ran out of `RESPONSE_BUDGET`. Counted as failures AND counted here, so a
161+
/// window whose failures are really our timeout can be recognised as one.
162+
budget_exceeded: u64,
155163
lat: Vec<u64>,
156164
}
157165

158166
/// One connection-holder's request loop. Opens a connection and reuses it, reconnecting on failure,
159167
/// because a fresh TCP handshake per request would measure the kernel rather than the gateway.
160168
async fn worker(addr: SocketAddr, req: Arc<String>, stop: Arc<AtomicBool>) -> WorkerStats {
161-
let mut w = WorkerStats { ok: 0, fail: 0, rig_refused: 0, lat: Vec::with_capacity(1024) };
169+
let mut w = WorkerStats { ok: 0, fail: 0, rig_refused: 0, budget_exceeded: 0, lat: Vec::with_capacity(1024) };
162170
let mut conn: Option<TcpStream> = None;
163171
// ALLOCATED ONCE, reused across every request this task sends: a fresh Vec per response would
164172
// put an allocator call in the timed hot path of every exchange, for a task that runs for the
@@ -235,6 +243,13 @@ async fn worker(addr: SocketAddr, req: Arc<String>, stop: Arc<AtomicBool>) -> Wo
235243
Exchange::ClosedBeforeAnyBytes if !fresh => {
236244
conn = None;
237245
}
246+
// Out of budget is a failure the caller would have felt, and also a bound of ours, so it
247+
// is counted in both places rather than hidden in one.
248+
Exchange::BudgetExceeded => {
249+
w.fail += 1;
250+
w.budget_exceeded += 1;
251+
conn = None;
252+
}
238253
// The same thing on a connection opened moments ago is the target refusing to answer.
239254
Exchange::ClosedBeforeAnyBytes | Exchange::Failed => {
240255
w.fail += 1;
@@ -278,8 +293,18 @@ enum Exchange {
278293
/// stale connection - the peer closed an idle one - not a failed request, because the request
279294
/// never reached a server that was listening. On a FRESH connection it is a real failure.
280295
ClosedBeforeAnyBytes,
281-
/// A genuine failure: a non-2xx, a malformed head, a truncated body, a timeout.
296+
/// A genuine failure: a non-2xx, a malformed head, a truncated body.
282297
Failed,
298+
/// The request did not complete inside `RESPONSE_BUDGET`.
299+
///
300+
/// STILL A FAILURE - a gateway that cannot answer one request in thirty seconds has failed any
301+
/// caller that was waiting - but a failure whose bound is OURS, and the artifact has to be able
302+
/// to say so. Every other rig limit that ever bound was invisible for exactly this reason: it
303+
/// landed in the same counter as a genuine refusal and nothing downstream could separate them.
304+
/// The budget is roughly a thousand times the slowest gateway this field has measured, so this
305+
/// should never fire; "should never fire" is what was true of the ephemeral port range until it
306+
/// did.
307+
BudgetExceeded,
283308
}
284309

285310
/// Whether the peer announced it will close after this response.
@@ -357,7 +382,7 @@ async fn read_response(s: &mut TcpStream, deadline: Instant, acc: &mut Vec<u8>)
357382
// deadline-minus-a-moment, then the read blocks for its own full timeout.
358383
let remaining = deadline.saturating_duration_since(Instant::now());
359384
if remaining.is_zero() {
360-
return Exchange::Failed;
385+
return Exchange::BudgetExceeded;
361386
}
362387
let n = match tokio::time::timeout(remaining, s.read(&mut buf)).await {
363388
// Nothing arrived at all. The caller decides what that means: on a reused connection it
@@ -468,6 +493,7 @@ pub fn run(cfg: &GenConfig) -> GenStats {
468493
g.ok += w.ok;
469494
g.fail += w.fail;
470495
g.rig_refused += w.rig_refused;
496+
g.budget_exceeded += w.budget_exceeded;
471497
g.latencies_us.extend_from_slice(&w.lat);
472498
}
473499
// A task that panicked is a HARNESS fault, and its requests are gone. Say so on
@@ -498,6 +524,7 @@ mod tests {
498524
latencies_us: lat.to_vec(),
499525
spawn_failed: false,
500526
rig_refused: 0,
527+
budget_exceeded: 0,
501528
p50_us: None,
502529
p99_us: None,
503530
}
@@ -850,4 +877,44 @@ mod tests {
850877
// regression guard against collapse, not a performance assertion.
851878
assert!(wall < Duration::from_secs(20), "a 500ms window at c=2048 took {wall:?}; the holders are not cheap");
852879
}
880+
881+
// A RIG LIMIT MUST NEVER BE INDISTINGUISHABLE FROM A GATEWAY FAILURE.
882+
//
883+
// Both of these used to land in `fail` beside a genuine refusal, where nothing downstream could
884+
// separate them - which is how the search came to publish this host's ephemeral port range as a
885+
// gateway's ceiling. They are counted apart now, and the stats line carries them across the
886+
// subprocess boundary because that line is everything the parent learns from the child.
887+
#[test]
888+
fn the_stats_line_carries_our_own_limits_apart_from_the_gateways_failures() {
889+
let g = GenStats {
890+
ok: 100,
891+
fail: 7,
892+
elapsed_s: 1.0,
893+
spawn_failed: false,
894+
rig_refused: 3,
895+
budget_exceeded: 2,
896+
latencies_us: vec![1000, 2000, 3000],
897+
p50_us: None,
898+
p99_us: None,
899+
};
900+
let line = g.stats_line();
901+
assert!(line.contains("rigrefused=3"), "our port/descriptor exhaustion must cross the wire: {line}");
902+
assert!(line.contains("budgetexceeded=2"), "our response budget must cross the wire: {line}");
903+
assert!(line.contains("fail=7"), "and the gateway's own failure count is unchanged: {line}");
904+
905+
// The parent reads them back.
906+
let parsed = crate::loadgen::parse_ugen_line(&line).into_value().expect("a line this generator wrote must parse");
907+
assert_eq!(parsed.rig_refused, 3);
908+
assert_eq!(parsed.budget_exceeded, 2);
909+
assert_eq!(parsed.fail, 7);
910+
911+
// A line from an older generator has neither field and must still parse: absent means "not
912+
// reported", which is not the same as "did not happen", and refusing an otherwise complete
913+
// line would turn a cosmetic version skew into a lost measurement.
914+
let old = "rps=1000 fail=1 p50=1.00 p99=2.00 p50us=1000 p99us=2000 ok=999";
915+
let legacy = crate::loadgen::parse_ugen_line(old).into_value().expect("an older line must still parse");
916+
assert_eq!(legacy.rig_refused, 0);
917+
assert_eq!(legacy.budget_exceeded, 0);
918+
assert_eq!(legacy.ok, 999);
919+
}
853920
}

‎engine/src/loadgen.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ pub struct UgenStats {
3939
/// simply has no such count, and defaulting to 0 is right - it means "none reported", not "none
4040
/// happened", and the alternative is refusing to parse a line that is otherwise complete.
4141
pub rig_refused: i64,
42+
/// Requests that exhausted the generator's response budget. Optional on the wire for the same
43+
/// reason as `rig_refused`.
44+
pub budget_exceeded: i64,
4245
}
4346

4447
fn parse_ugen_fields(line: &str) -> Result<UgenStats, String> {
@@ -50,6 +53,7 @@ fn parse_ugen_fields(line: &str) -> Result<UgenStats, String> {
5053
p99_us: require_i64(&fields, "p99us", line)?,
5154
ok: require_i64(&fields, "ok", line)?,
5255
rig_refused: fields.get("rigrefused").and_then(|v| v.parse::<i64>().ok()).unwrap_or(0),
56+
budget_exceeded: fields.get("budgetexceeded").and_then(|v| v.parse::<i64>().ok()).unwrap_or(0),
5357
})
5458
}
5559

@@ -119,7 +123,7 @@ mod tests {
119123
let m = parse_ugen_line(UGEN_LINE);
120124
assert_eq!(
121125
m,
122-
Measurement::Measured(UgenStats { rps: 1234, fail: 3, p50_us: 12_500, p99_us: 45_000, ok: 14_808, rig_refused: 0 })
126+
Measurement::Measured(UgenStats { rps: 1234, fail: 3, p50_us: 12_500, p99_us: 45_000, ok: 14_808, rig_refused: 0, budget_exceeded: 0 })
123127
);
124128
}
125129

‎engine/src/run.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,23 @@ impl Probe for SweepProbe<'_> {
338338
eprintln!("loadgen: could not reach c={concurrency}; the rig refused a thread");
339339
return None;
340340
}
341+
// THE RIG RUNNING OUT IS NOT A GATEWAY RESULT, and it is the same class of fact
342+
// `spawn_failed` already models: the window never ran at the concurrency it claims, so
343+
// nothing about the gateway was learned at any concurrency we could name.
344+
//
345+
// These are connections THIS HOST could not make - ephemeral ports or descriptors exhausted
346+
// (EADDRNOTAVAIL/EMFILE). They used to land in `fail` beside a genuine refusal, so the gate
347+
// failed and the search recorded our own port range as the gateway's ceiling. Counting them
348+
// separately was only half the fix: a window containing any of them still failed, and still
349+
// failed for our reason. It is unmeasured instead.
350+
if stats.rig_refused > 0 {
351+
eprintln!(
352+
"loadgen: could not reach c={concurrency}; this host refused {} of its own connections \
353+
(ephemeral ports or descriptors exhausted) - the window never ran at that concurrency",
354+
stats.rig_refused
355+
);
356+
return None;
357+
}
341358
// A window that produced nothing is UNMEASURED, not a zero.
342359
if stats.ok == 0 && stats.fail == 0 {
343360
return None;
@@ -448,6 +465,7 @@ pub fn load_window_at(
448465
latencies_us: Vec::new(),
449466
spawn_failed: false,
450467
rig_refused: u.rig_refused.max(0) as u64,
468+
budget_exceeded: u.budget_exceeded.max(0) as u64,
451469
// The subprocess never sends its raw samples back, only the percentiles it already
452470
// computed over them, so these are filled straight from the stats line rather than left
453471
// for a caller to (wrongly) derive from the now-empty `latencies_us` above.
@@ -618,6 +636,23 @@ impl Probe for SustainedProbe<'_> {
618636
eprintln!("loadgen: could not reach c={concurrency}; the rig refused a thread");
619637
return None;
620638
}
639+
// THE RIG RUNNING OUT IS NOT A GATEWAY RESULT, and it is the same class of fact
640+
// `spawn_failed` already models: the window never ran at the concurrency it claims, so
641+
// nothing about the gateway was learned at any concurrency we could name.
642+
//
643+
// These are connections THIS HOST could not make - ephemeral ports or descriptors exhausted
644+
// (EADDRNOTAVAIL/EMFILE). They used to land in `fail` beside a genuine refusal, so the gate
645+
// failed and the search recorded our own port range as the gateway's ceiling. Counting them
646+
// separately was only half the fix: a window containing any of them still failed, and still
647+
// failed for our reason. It is unmeasured instead.
648+
if stats.rig_refused > 0 {
649+
eprintln!(
650+
"loadgen: could not reach c={concurrency}; this host refused {} of its own connections \
651+
(ephemeral ports or descriptors exhausted) - the window never ran at that concurrency",
652+
stats.rig_refused
653+
);
654+
return None;
655+
}
621656
// A window that produced nothing is UNMEASURED, not a failing rung.
622657
if stats.ok == 0 && stats.fail == 0 {
623658
return None;

0 commit comments

Comments
 (0)