feat(debug-trace-server): inbound admission control and response-size caps - #188
Conversation
… caps Nothing bounded what clients could ask of this process. Every existing concurrency cap is outbound — the witness, data and R2 semaphores limit what we ask of others, and they are unbounded waits, so overflow could only ever surface as a timeout. EVM tracing runs inline on the runtime's worker threads, so enough concurrent requests starve chain sync, the accept loop and the metrics exporter along with each other. Requests beyond the configured budget are now refused immediately with -32013 "Request queue is full", matching mega-reth's ConcurrencyLimiter byte for byte so existing client backoff applies unchanged. The gate is split in two, and the split is load-bearing rather than stylistic. AdmissionLayer sits inside ConcurrentBatchLayer — which decomposes batches into per-entry calls rather than delegating to an inner batch, so an inner layer sees single calls and every batch entry — and only ever runs a non-blocking CAS against max_concurrent + max_queue. The execution permit is taken in the handler, after the response cache misses. CancelGuard arms on a request's first poll and the handlers record their arrival synchronously in that same poll, so a middleware gate that parked before the handler would record a cancellation with no matching arrival for every client that hung up while queued: negative, permanent drift in the accounting identity, worst under exactly the overload the gate exists for. A layer that only CAS-es keeps arm and arrival in one poll, and the permit wait then sits after the arrival is already booked. The placement pays three more ways: a cache hit never takes a permit, the typed RequestShape is already in hand so the heavy-tracer sub-cap costs no second parse of attacker-controlled JSON, and what the permits count is blocks actually being fetched and replayed. Each handler mints its deadline once and passes it to both the permit wait and the fetch, so the queue is carved out of --block-fetch-timeout rather than added on top of it. The reserve held back for the witness stage falls back to half the budget when --witness-timeout does not fit inside --block-fetch-timeout, a legal pair that would otherwise leave a zero-length wait and silently reduce --admission-max-queue to a no-op. Two bugs in the reference implementation are deliberately not carried over: its wait future is created after the capacity check, so a permit freed in that window is never observed, and raising a limit never wakes parked waiters. Both are avoided by using a resizable FIFO Semaphore — grow with add_permits, shrink with forget_permits plus a debt counter settled on release — and both directions are regression-tested. Occupancy is counted rather than derived from available permits: once a shrink leaves debt behind, that difference reports the new limit, so the admin RPC would have answered a retune by claiming it had already taken effect while every old holder was still resident. Every struct-logger request is heavy, including the bare default one. It emits a record per executed opcode, and the only thing separating it from its already- heavy flagged sibling is a flag that changes the size of that output rather than its kind — so an opts-less debug_trace* call is limited by --admission-heavy-max-concurrent. --max-response-size is checked at this server's own serialization point, so an over-limit body is dropped before it can be copied again into the JSON-RPC envelope; --max-batch-response-size separately caps the assembled batch, because a batch retains every completed entry's body until it finishes and that accumulation, not any single response, is what has previously exhausted this process. Startup logs both the heavy and the overall concurrency-times-size products; neither bounds a tracer's intermediate allocations, and the doc says so. --admin-addr starts a second listener, loopback enforced, on its own thread and runtime — inline tracing on the main runtime would otherwise starve it exactly when it is needed — serving admin_getConcurrencyLimit / admin_setConcurrencyLimit so the limits can be retuned without a restart. It carries no RPC middleware, so a saturated public port cannot shed the call that relieves it. A shed records the balanced pair shape="shed" / reason="overloaded" from inside the request future, where the ERROR_SELF_REPORTED task-local suppresses settle_response's unattributed drift arm; settle_response itself is unchanged. Verified on a live server: 30 concurrent requests against a 16-capacity gate shed exactly 14, the balanced pair matched exactly, the per-method identity summed, and the drift alarm stayed at zero; shrinking maxConcurrent 4->1 with four permits held reported 4 executing and agreed with the Prometheus gauge. The layer-order pin, the task-local claim, and the occupancy fix were each mutation-tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude review status
✅ Review clean Last reviewed: New this round: 0 finding(s), 0 question(s) · Resolved this round: 1 · Open questions: 0 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 129d043b22
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d budget Three findings from the Codex review of #188, all reproduced before fixing. The admitted budget is class-blind by design — the gate runs before anything parses the tracer — so heavy requests were admitted against `max_queue + max_concurrent` and then bottlenecked on a sub-cap a fraction of its size. A flood of them filled the budget while barely executing: reproduced with 12 heavy requests against a 12-slot gate, where 1 executed, 3 execution permits sat idle, and an ordinary request was refused. That is a priority inversion handed to whoever sends the most expensive shape, in the feature meant to prevent exactly that. Heavy requests now get their own share — they may queue in the same proportion to their execution budget as the process as a whole — which also makes `--admission-max-queue 0` mean execute-or-shed for them, where a share of the shared budget let 632 of them wait at the defaults. The heavy occupancy gauge was raised only once both permits were in hand, so it read zero while every sub-cap permit was reserved and further heavy requests were blocked on them — disagreeing with the admin RPC, which counted them immediately. It is now raised by a guard at acquisition, the same shape the ordinary occupancy counter already uses. `--max-batch-response-size` was allowed to equal `--max-response-size`, but our own check measures the bare body while the framework's cap measures the envelope and client `id` too. A body that just passed ours could still be swapped for an oversized-response error after the handler had counted the request as served, and without incrementing the oversized counter. Startup now requires headroom between the two caps and names why, and the framework-swapped case is counted on `debug_trace_response_oversized_total` so that series is complete either way. Both heavy-budget regressions were mutation-tested: restoring the shared budget fails them, and the zero-queue case asserts promptness rather than outcome, since the old behaviour also ended in `Overloaded` — just after parking until the deadline, which is the queueing that configuration says it does not want. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cd41340b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Two more findings from the Codex re-review of #188; a third was refuted. Shrinking a saturated limiter recorded the debt only after `forget_permits` reported how little it could take. A release landing between the two observed zero debt, handed its permit back, and let a queued request through above the limit the shrink had just set — corrected only by a later release. Publishing the debt first inverts the window: a release now sees a debt that is at worst too large and forgets its permit, which is the conservative direction, and the part that could be removed immediately is cancelled afterwards. The accounting is unchanged in the quiescent case. The envelope headroom between `--max-batch-response-size` and `--max-response-size` was a fixed 64 KiB, which is a guess rather than a bound: the envelope's only unbounded part is the client-supplied `id`, and a client can pad one well past that. It is now derived from the request-body cap, which bounds the id outright, and that cap is pinned explicitly rather than inherited from the framework's default so the derivation cannot drift on an upgrade. Refuted: gating notifications. `RpcService::notification` answers without dispatching the method at all, so a notification reaches no handler, waits on no permit and does no work; gating one would spend capacity on nothing, and since a notification carries no response a shed could only be silent. Pinned by `a_notification_never_reaches_a_handler_or_takes_capacity`, which asserts the handler never runs and no capacity is taken even with the gate saturated. The new debt stress test guards end-state accounting — no permit lost to a double-forget, none conjured by a release that should have forgotten one — and is bounded by a timeout so a protocol that stops circulating permits fails red instead of hanging the job. It deliberately does not claim to pin the reordering above: the old ordering's defect is a transient over-admission that a quiescent invariant cannot observe, and it still passes under that mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 558594ed14
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…plify Quality pass over #188 from four independent review angles (reuse, simplification, efficiency, altitude). No behaviour change; net -92 lines despite adding a helper, a test and docs. Gating is now derived rather than enumerated. `GATED_METHODS` was `ALL_METHODS` minus the cache-status endpoint, and `main.rs` carried a second `EXEMPT` list that also had to spell the `timed_` alias, so one fact lived in three places and a startup panic existed to police them. There is now a single `GATE_EXEMPT_METHODS`, `is_gated` derives from it, and the startup check asks something with independent value instead: whether a registered method is absent from `ALL_METHODS`, which would both ungate it and collapse its metrics onto `unknown`. The bounded compare-and-swap that admits a request was written twice, once per counter, with the memory orderings — the subtle part — copied. It is now one `try_claim`. `acquire_by` became an inherent method so the `checked_out` increment sits next to the decrement it pairs with. The rule that a gate must cover one batch's concurrent entries was enforced at startup and again on the admin setter, with independently worded messages. Both now call one predicate that takes each caller's spelling for the limits, the same shape and for the same reason as `stateless_common::R2Flag`. `TraceWeight` moved next to `RequestShape`, which produces it, so `response_cache` no longer imports from `admission`; the gate consumes the classification rather than owning it. `acquire_execution` takes the type instead of a `bool`, so the class survives to the mechanism and a future third class stays internal to `admission`. `trace_block_by_hash` open-coded the cache-check then permit then fetch sequence that `lookup_block_by_number` already encapsulates; it now goes through a by-hash sibling. The tx handlers keep their two inline lines deliberately — sharing them would need an error type spanning admission refusal and fetch failure just so the Parity path can keep degrading the latter to `null`, which is more machinery than it removes. Smaller: one construction site for the shed error (and `borrowed`, so the path that runs under strain allocates nothing); `admin` reuses the crate's `invalid_params_err`; the admin listener's three-deep send-and-return ladder became one `?` chain; `check_response_size` returns the message it already formatted instead of a struct existing to be stringified; `permit_reserve` is a free function, so its test asserts three subtractions instead of standing up an RPC client three times; and the admin tests use `#[tokio::test]` and the crate's HTTP helpers rather than a hand-rolled runtime driver and a second blocking client. Skipped, with reasons: caching `AdmissionMetrics` handles (measured at 143 ns on a path that does a network fetch and an EVM replay, and it would introduce a second handle-construction pattern into a file that consistently uses one); threading the method label down from the batch layer (measured cheaper to recompute at 1 ns than to carry through request extensions); moving the metrics exporter onto the admin runtime (a real gap — it is starved by the same inline tracing the admin listener was isolated from — but a behaviour change to a pre-existing subsystem, so it belongs in its own change); and extracting a `request_shape` module (the wrong-way dependency it targets is already gone). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd26e256ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…n seams Four review angles (reuse, simplification, efficiency, altitude) over #188 again, after bd26e25. No behaviour change; net -17 lines. The shed arrival series, the occupancy gauges, and the oversized-response counter are all per-gated-method series, but only two of them derived that set from gated_methods() — the shed pre-registration was three hand-kept lists whose sync-by-comment had already drifted (it cited GATED_METHODS, a symbol the derived-gating rename removed). All three now pre-register from one gated_methods() loop, gated_methods() itself filters through is_gated instead of re-spelling the exemption, and the stale GATED_METHODS references in main.rs and AGENTS.md now describe the check that actually runs. lookup_block_by_hash is inlined into its one caller. Its comment claimed the cache -> permit -> fetch order was shared per handler, but both tx handlers derive it inline; the helper cost a six-argument signature plus an enum round-trip to save ten lines. Helpers now exist exactly where two callers share one (lookup_block_by_number). An ungated call now hands back the inner service's future untouched instead of paying a service clone and a wrapper block to pass through — the idiom ConcurrentBatch::call already documents — and the shed path moves the request id into the error response rather than cloning it. The shed recording stays inside the returned future, where the task-local scope lives. Test hygiene: far() moves to rpc_middleware::test_support (it carried an unreachable pub(crate) while four other test modules re-spelled the cutoff), the drain_into alias over drain() is gone, the env-var cases each carry their own field accessor instead of re-matching on the variable name, and the notification test's static flag becomes a local Arc. The admin setter gains a comment pinning why its validate-then-apply section must stay await-free on the current-thread admin runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![mega-maxwell[bot]](https://avatars.githubusercontent.com/u/138558126?s=60&v=4){kind=small}
… framework response ceiling Two review findings on #188, one from each bot. A release landing between a shrink's debt publish and its permit removal pays one unit of debt by forgetting a permit forget_permits was about to remove anyway, so the old settlement removed one permit too many — permanently, since growth only re-adds the difference between limits. Repeated retunes under load slid real capacity below the configured value with no error surface and only a restart to recover. The settlement now compares what the removal took against what the debt could still absorb and returns the overlap, keeping the release path lock-free. The window is nanoseconds wide, so the regression drives it through a test-only hook rather than racing tasks at it; the mutant without the compensation fails the test with exactly the predicted off-by-one. --max-response-size must now fit, with envelope headroom, under the JSON-RPC framework's u32 response ceiling, rejected at startup by name: above ~4 GiB the framework's clamped cap silently undercut our own check, so bodies between the two passed it and were then swapped for the framework's oversized-response error after being counted served. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Approving. Nothing I found is blocking.
Verified independently, rather than taken on trust:
-
The notification pass-through is correct and the P1 against it does not hold.
RpcService::notificationinjsonrpsee-server-0.26.0/src/middleware/rpc.rs:190-195returnsMethodResponse::notification()without ever resolving a method, andRpcService::batchroutesBatchEntry::Notificationthrough that same method. No handler runs, so there is no work to gate. -
The
ResizableSemaphoreresize is sound. I workedavailable + checked_out == limit + debtthrough acquire, release (both branches), grow and shrink individually — each preserves it, so any interleaving does. I also traced four specific orderings to completion: the original10 → 5counterexample, a window containing more releases thanwanted, a shrink belowchecked_out, and a release landing betweenlimit.swap()anddebt.fetch_add()(that last window is not covered by any of the review threads; it is safe, because the returned permit is reclaimed by the subsequentforget_permits).removed - cancelledis the right compensation. -
Gate coverage is complete.
trace_blockandtrace_parity_transactionboth take execution permits (the former via the permit returned fromlookup_block_by_number, the latter directly). Derivingis_gatedfromALL_METHODSand failing startup inassert_admission_covers_modulemakes the "new method silently ungated" failure structurally unreachable — the strongest thing in this diff.
Non-blocking, in rough order of how much I would care:
-
debug_trace_admission_permit_wait_secondsonly records on success —self.execution.acquire(cutoff).await?returns before the histogram observation, and the heavy acquisition does the same. Requests that wait out the cutoff contribute no sample, so under overload the histogram drops exactly its longest waits. The README points at this metric as the input for tightening the defaults; anoutcomelabel or an observation on the timeout path would make it answer that question. -
--max-response-size(256 MB) is new —serialize_replyonmaintakes no size argument and there is no single-response cap in the base at all. Over-limit bodies are now dropped with aTraceFailederror. That is clearly intentional given the OOM that motivated the PR, but it is the one change here that can turn a request that succeeds today into a failure, and it is not in the "Behaviour changes to be aware of" list alongside the other two. Worth a line there. -
max_connectionsis left at the framework default (100) whilemax_request_body_sizeandmax_response_body_sizeare both set explicitly. It appears twice in the deferral rationale as "the real bound is connections x body size", so it is load-bearing for two follow-ups while being the one value in that product nobody configured. Setting it explicitly would put it on the record. -
heavy_capacity()'smax_queue / max_concurrentis integer division: any configuration withmax_queue < max_concurrentcollapsesqueue_per_slotto 0 and silently turns the heavy class into execute-or-shed. Fine at the defaults (8192/640 = 12); indistinguishable from the deliberate--admission-max-queue 0semantics if an operator lowers the queue. -
CallTracer(_)isNormalwhileDefaultisHeavyon the reasoning that a config flag "changes its size, not its kind" —callTracer'swithLogis the same shape of flag. Presumably an order-of-magnitude difference justifies it; worth saying so in the comment, or the next reader will apply the struct-logger argument here. -
BatchRequestConfigis left at the defaultUnlimited. Entries admit individually so this is not a bypass, but given that batches are the shape named as having taken this server down, and that the two neighbouringServerConfiglimits are already pinned explicitly, this one seems worth pinning too.
The design reasoning is unusually well-argued, and the responses on the review threads are too — including the one that pushes back, which checks out.
3 participants
Summary
Adds inbound admission control to
debug-trace-server, plus the response-size caps it depends on. Requests beyond a configured capacity budget are now refused immediately with-32013 "Request queue is full"— byte-identical to mega-reth'sConcurrencyLimitercontract, so whatever already backs off for the node backs off for this server. Lands backlog items P0-0 PR③ and P1-1 (TODO-H).Root cause
Nothing bounded what clients could ask of this process. Every existing concurrency cap is outbound — the witness, data and R2 semaphores limit what we ask of others, and they are unbounded waits, so overflow could only ever surface as a timeout. EVM tracing runs inline on the runtime's worker threads (
tracing_executor.rshas no blocking pool), so enough concurrent requests starve chain sync, the accept loop and the metrics exporter along with each other. Two production incidents already followed from this: an OOM kill from concurrently materialised largeprestateTracerresponses, and a co-located node degraded ~15× by concurrent cold reads.Design — the gate is split in two, and that is load-bearing
AdmissionLayer(admission.rs) sits insideConcurrentBatchLayerand does only a non-blocking CAS againstmax_concurrent + max_queue. The execution permit is taken in the handler instead, after the response cache misses (rpc_service.rs:267).A single gate in the middleware would break the metrics accounting identity.
CancelGuardarms on a request's first poll (rpc_middleware.rs:242) and the handlers record their arrival synchronously in that same poll —trace_block_by_number:560→classify_and_gate→record_request_shape:353, with the first.awaitonly at:570. Arm and arrival are atomic. Anything that parks in between makes a client hangup record a cancellation with no matching arrival, i.e. permanent negative drift, worst under exactly the overload the gate exists for. A layer that only CAS-es preserves that by construction, and the permit wait then sits after the arrival is already booked, so a hangup there balances for free.Placement inside the batch layer is also what makes batch entries admit individually:
ConcurrentBatch::batchnever delegates to an innerbatch, it decomposes into per-entryservice.call(rpc_middleware.rs:292). Reversing the two.layer()calls silently lets whole batches through ungated, sobatch_entries_are_individually_gatedpins the order.Three further consequences: a cache hit never takes a permit; the typed
RequestShapeis already in hand, so the heavy-tracer sub-cap costs no second parse of attacker-controlled JSON; and what the permits count is blocks actually being fetched and replayed.Sizing
Defaults are derived from the most recent capacity run rather than from a constant. Recomputing that run by Little's law —
request_duration_seconds_sum 51,549s ÷ 87.65s wall— gives 588 blocks inside handlers, not the 1,766 in-flight figure that run reported; the latter is batch-arrival occupancy, and the 3.0× gap is exactly--batch 30 ÷ --batch-item-concurrency 16. They size different knobs, so--admission-max-concurrentdefaults to 640 (just above the measured 588) and--admission-max-queueto 8192. Both sit above every clean measurement: no saturation point has been established for this workload, and a default that throttles a known-good one is the worse failure. Tighten oncedebug_trace_admission_in_flightanddebug_trace_admission_permit_wait_secondsshow what production does.The binding resource is the upstream, not CPU: across that run's concurrency ladder, upstream concurrency rose 97× while
eth_getBlockByHashmean went 3.9 ms → 125.2 ms (32×), EVM moved 1.2× and request-path CPU 1.15×.Two bugs in the reference implementation, deliberately not carried over
mega-reth's
concurrency_limiter.rs:260-283creates itsNotifiedafter the capacity check, so a permit freed in that window is never observed and the request parks until some unrelated request finishes — at the tail of a burst, indefinitely. And its setters (:189-191) are a barestore, so raising a limit never wakes parked waiters; combined with an acceptedmax_concurrent = 0that is unrecoverable without a restart. Both are avoided by using a resizable FIFOtokio::sync::Semaphore(grow viaadd_permits, shrink viaforget_permitsplus a debt counter settled on release), and both directions are regression-tested. Note the comment there claiming aSemaphorecannot shrink is out of date as of tokio 1.36.Behaviour changes to be aware of
Every struct-logger request is heavy, including the bare default. A
debug_trace*call with notraceremits a record per executed opcode, and the only thing separating it from its already-heavy flagged sibling is a flag that changes the size of that output, not its kind. So an opts-less call now passes--admission-heavy-max-concurrent(default 8). If tracerless calls are a normal part of the workload, raise that flag.--max-batch-response-size(default 1GB) now bounds batch assembly, previously pinned tou32::MAX. It is deliberately a separate knob from--max-response-size(256MB): a batch retains every completed entry's body until the batch finishes, so its memory is the sum of its entries, and that accumulation — not any single response — is what has exhausted this process before. Reusing one value for both would have capped whole batches at the single-response limit.Testing
cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features(no warnings),cargo sort --check, andcargo test --workspace(466 tests) all clean.Verified against a live server rather than only in tests: 30 concurrent requests at a 16-capacity gate shed exactly 14;
shape="shed"=reason="overloaded"= 14; the per-method identity summed exactly (shape 30 = served 0 + errors 30 + cancelled 0) withreason="unattributed"at zero. The 16 admitted requests died ondeadline_blockagainst a deliberately-unreachable upstream while the 14 refused were refused promptly — the whole design in one measurement. ShrinkingmaxConcurrent4→1 over the admin RPC with four permits held reported 4 executing and agreed with the Prometheus gauge.Three claims were mutation-tested rather than assumed: swapping the two
.layer()calls makesbatch_entries_are_individually_gatedfail; bypassingrecord_rpc_errorin the shed path makes sheds land on the drift alarm; revertingexecuting()to the derived form makes the occupancy test report 1 where 4 are running.Notes
A 24-agent adversarial review of the diff raised 20 findings, 9 of which survived independent refutation and are all fixed here. The most consequential:
executing()was derived aslimit - available_permits, so once a shrink left debt behind it reported the new limit as occupancy — meaningadmin_setConcurrencyLimitwould have answered a retune by claiming it had already taken effect while every old holder was still resident. Also fixed: by-hash and tx handlers minted the fetch deadline twice, so the queue wait was added on top of--block-fetch-timeoutinstead of carved out of it; and a legal--witness-timeout >= --block-fetch-timeoutpair silently reduced--admission-max-queueto a no-op.Deliberately out of scope, and stated as such in the docs rather than papered over: nothing here bounds a tracer's intermediate allocations, so the logged concurrency × size products are not a full resident-set bound — a per-transaction-count gate (TODO-G's deeper half) would be. Also unimplemented is the adaptive tier the capacity work argues for (measured service rate → predicted queue wait); this PR is a pure hard gate by design.
Before any overload benchmarking, the load generator needs two fixes: its open-loop
--ratemode ignores--batch, and its verdict function scores a shed response as a failure, so every overload test would report FAIL.