fix(agentx): close AgentX round timeout gaps + overhead-cap visibility - #1309
Open
zengleixin-amd wants to merge 11 commits into
Open
fix(agentx): close AgentX round timeout gaps + overhead-cap visibility#1309zengleixin-amd wants to merge 11 commits into
zengleixin-amd wants to merge 11 commits into
Conversation
All three were found by running the AgentX path end to end on GLM-5.2-MXFP4 and DeepSeek-V4-Pro (8x MI355X, TP=8, full 062126 corpus). Each one silently cost a multi-hour round. 1. Warmup was missing from the non-canonical list. The agentic warmup is what puts the cache under realistic pressure before the window opens, so 1 request/lane instead of 10 measures a materially emptier cache. aiperf has no notion of "enough warmup", so such a round came back submission_valid=true and looked publishable. On a 743B model the canonical 10/lane is a ~2h warmup, which is exactly when an operator reaches for the knob -- the hole was reachable in practice, not theoretical. Both AGENTX_WARMUP_REQUESTS_PER_LANE and AGENTX_WARMUP_GRACE_PERIOD now register as deviations. 2. The self-bracketed profile delay could open after the round had ended. AGENTX_PROFILE_WARMUP_S is a blind wall clock with no idea which phase aiperf is in, and the two failure directions are not symmetric: opening late yields NO trace (aiperf exits, the branch only warns), opening early yields a capture of a still-loaded system that TraceLens can use. Observed both ways on the same model in one day. Clamp toward early at DURATION - window - 60s and say so when the clamp bites. 3. The profile capture bound is calibrated on the synthetic shape. 128 decode steps is serialization-safe at 1024/1024; an agentic step carries a measured ISL p50 of 56k-96k tokens. On DeepSeek-V4-Pro that put each of the eight vLLM workers at 113-127 GB of HOST RAM -- Ray reported 1012/1024 GB and killed the capture, three attempts running, so the roofline arm produced no trace at all. AgentX now caps at 8; the client bounds the window by wall clock anyway, so the extra steps only inflate the in-memory event buffer. HYPERLOOM_PROFILE_MAX_ITERS still overrides. Tests: three new cases in test_aiperf_client_sh.py, negative-verified by reverting the client fix (the two "must flag" cases go red, the no-false-positive case stays green). Full file: 37 passed on a POSIX box. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d cap Found by running the AgentX path end to end with no phase disabled -- the first time that has been done, because every earlier run in this campaign passed --no-kernel and friends. A GLM-5.2 variant launched 09:47:41 died at 11:47:41.575: twenty-plus connections dropped in the SAME millisecond while the server was healthy and still prefilling with 55 requests running. A simultaneous mass disconnect against a live server is a subprocess kill, not a workload problem -- but aiperf treats a cancelled root warmup credit as terminal, so it surfaces as `warmup_failure` and the real cause never appears in the abort reason. Every variant cap is sized for the synthetic 1024/1024 shape: 7800s for integrate, 2400s for explore, 1800s for the conc sweep. A canonical AgentX warmup is 10 requests per lane over real agentic traces and runs past two hours on a 700B-class model before the measured round begins, so those caps kill the round by arithmetic rather than by chance. baseline already derives an AgentX-aware cap; only that path got it. Raised at the single choke point every variant round resolves through, reusing baseline's own resolver so there is not a second number to keep in sync, and never lowering a cap an operator asked for. AgentX is an opt-in benchmark branch: the gate means that with it off this is a no-op and the default path is untouched -- asserted directly, including with stale AGENTX_* vars present. This also retires three hypotheses that looked right and were not: concurrency contention (canonical conc=64 clears baseline with zero errors), a mis-passed --warmup-grace-period (removing it still failed at 706/707), and poisoned corpus entries (the recurring trace ids are an artefact of seed 42 fixing dispatch order; their sizes are the 9th and 88th percentile). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second half of the variant-cap defect, found by running the fixed path further. Raising the variant cap carried GLM-5.2 to the roofline round for the first time, and then rounds kept dying with `warmup_failure` anyway. The reason is not in the round: a Qwen3.8 baseline whose server answered all 685 chat/completions with 200 was cut at exactly its 7200s timeout, mid-warmup, after which the client could no longer connect. The log names it plainly -- `baseline_executor: timeout=7200s (explicit task param)`, and 9000s elsewhere. Integrate passes an explicit timeout_sec, sized for the synthetic shape, and a canonical AgentX warmup (10/lane over real agentic traces) does not fit either value. aiperf reports the cancelled warmup credit as `warmup_failure`, so the timeout never appears in the abort reason and it reads as a workload problem. Raised at the producer, not the consumer. `_resolve_timeout` deliberately lets an explicit param outrank the AgentX derivation and has a test pinning that contract; the first attempt here overrode it and broke test_explicit_task_param_still_outranks_agentx, which is exactly the kind of deliberate decision a test exists to defend. So this sits next to _cold_start_rebaseline_timeout, which was written for the very same reason: an explicit param suppresses the executor's own sizing branch. Gated on agentx_enabled(), never lowers a larger value, and follows baseline's own resolver so there is no second number to keep in sync. AgentX is an opt-in benchmark branch and must not move the default path -- asserted directly, including with stale AGENTX_* vars in the environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…r a model AGENTX_BASELINE_OVERHEAD_SEC's default (7200s) is calibrated on GLM-5.2/Qwen3.8 measurements. A raw aiperf run against Kimi-K3 at concurrency=64 measured warmup alone draining in ~12075s, already past the whole default cap before profiling even starts. Warn when the default is used unmodified so an operator sees this before the round is silently killed hours in, instead of only after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| """ | ||
| # Local import: baseline imports from this module, and the rest of the file | ||
| # already resolves _workload_envs this way. | ||
| from ._workload_envs import agentx_enabled |
|
|
||
| if not agentx_enabled(): | ||
| return cap | ||
| from .baseline import agentx_baseline_timeout_sec |
- baseline.py: treat an invalid AGENTX_BASELINE_OVERHEAD_SEC override (non-integer, zero, or negative) the same as unset, so the "using the default overhead" warning still fires instead of being silently suppressed by a broken value. - _grid_runner.py: _skip_rest_for_budget now gates admission on the AgentX-raised variant cap (agentx_variant_timeout_sec), not the declared one, when no per-variant estimate is available -- gating on the declared cap could admit a variant that the raised cap then gets clamped back down for, reproducing the mid-warmup kill this cap-raise exists to prevent. - _grid_runner.py: capture the measure round's actual granted timeout in a local variable and log that value on TimeoutExpired, instead of the outer variant_timeout_sec, so the timeout reported during diagnosis matches what was actually enforced. - _workload_envs.py: warn when AgentX's max-iters clamp lands below the steady-state floor, mirroring the existing warning on the manual HYPERLOOM_PROFILE_MAX_ITERS override path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- aiperf_client.sh: the _pmax safe bound for the self-bracketing profile delay only accounted for DURATION (the measurement window), not the warmup drain (bounded by WARMGRACE) that precedes it. On a long-warmup model this clamped an operator-tuned PWARM down to a fraction of what it needed to be, forcing the capture to fire mid-warmup -- the exact failure this self-bracketing exists to prevent. - aiperf_client.sh: WARMLANE/WARMGRACE non-canon checks used `!=`, so raising either above canonical (e.g. a longer grace period to let a large model's warmup fully drain) was flagged as a deviation even though it doesn't change what gets replayed. Changed to `-lt` so only a reduction is flagged. - aiperf_client.sh: CANON_WARMUP_PER_LANE/CANON_WARMUP_GRACE were declared twice with duplicated literals; now declared once, next to the WARMLANE/WARMGRACE defaults that derive from them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ate aware - request_handlers.py: _agentx_rebaseline_timeout only checked the ambient HYPERLOOM_AGENTX env var, so an integrate call driven from a subprocess that did not inherit it silently fell back to the non-agentic timeout. Now checks agentx_active(), which also honors the persisted benchmark_mode on SharedState. - _workload_envs.py: extracted agentx_active() as the shared persisted-state check (agentx_kb_write_blocked already had this logic inline) so both call sites agree on what "AgentX is on" means. - _workload_envs.py: when HYPERLOOM_PROFILE_MAX_STEPS_CAP is set explicitly and AgentX's clamp overrides it anyway, warn instead of silently overriding -- otherwise a deliberate operator setting appears to have no effect with no trace of why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion gaps Addresses the M1/M2/M3 findings of the PR 1309 architecture review. - baseline.py: AGENTX_BASELINE_OVERHEAD_SEC was a flat 7200s covering setup, corpus load, warmup and first-compile, calibrated on GLM-5.2/Qwen3.8. Warmup is the share that varies by model, and it already has an operator-visible bound in the client -- AGENTX_WARMUP_GRACE_PERIOD. A model whose warmup runs long is a model whose operator has already had to raise that knob, so the overhead now derives from it (5400s non-warmup + grace) instead of asking for a second, independent number meaning the same thing. At canonical settings the sum is unchanged, preserving the measured calibration. Every input is logged, and the "nothing has been tuned for this model" warning now fires only when neither knob is set. - _workload_envs.py: HYPERLOOM_PROFILE_MAX_ITERS is applied after the AgentX capture clamp and wins, which is intended -- but it lifts a host-RAM bound, not a serialization one, and neither existing warning could report it. `cap` defaults to _DEFAULT_PROFILE_MAX_STEPS (128), so the obvious override of 128 was neither below the steady-state floor nor above the cap and restored the full OOM exposure in silence. Still honoured verbatim; now warns. - aiperf_client.sh: AGENTX_FAILED_REQUEST_THRESHOLD was missing from the non-canonical list. Raising it keeps alive a run upstream's 0.10 would have aborted, and aiperf stamps no scenario marker for it, so the round came back submission_valid=true. Only a larger ratio is flagged; tightening it measures a strictly cleaner run. CANON_FRT is declared once and feeds the default, so the canonical value and the default cannot drift apart. - Cross-referenced the three synthetic-sized variant-timeout defaults with agentx_variant_timeout_sec, and added a CHANGELOG entry with operator notes for the two behaviour changes (derived cap, submission_valid on a raised failure threshold). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| # ISL/OSL shape: an AgentX round does not fit it, and is not meant to -- see | ||
| # ``agentx_variant_timeout_sec`` in ``_grid_runner``, which raises whatever cap | ||
| # reaches it rather than expecting this default to cover both workloads. | ||
| _VARIANT_TIMEOUT_SEC_DEFAULT = 7800 |
| # ISL/OSL shape: an AgentX round does not fit it, and is not meant to -- see | ||
| # ``agentx_variant_timeout_sec`` in ``_grid_runner``, which raises whatever cap | ||
| # reaches it rather than expecting this default to cover both workloads. | ||
| _VARIANT_TIMEOUT_SEC_DEFAULT = 7800 |
The AgentX-off counterpart of the MAX_ITERS override test asserts only on log records, so it never renders the materialized config and does not need yaml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI runs `ruff format --check .` (lint.yml), which this branch has been failing since 0c15594 -- the warmup non-canon assertions added there, and the failed-request-threshold ones added on top of them, were hand-wrapped rather than formatted. Formatting only, no behaviour change; the three files pass on a clean checkout (69 passed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… scrub TCP_USER_TIMEOUT bounds how long Linux tolerates an established connection making no progress, and an agentic turn against a long-context model makes none for as long as the server is prefill-bound. aiperf's stock 30s therefore aborts otherwise-live connections mid-prefill, which surfaces as a warmup failure with no server-side error to match it. Upstream's Kimi-K3 arms (benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh and the atom/b300 variants) and the DSv4 SGLang arms all export 900000 for exactly this reason -- the DSv4 comment names the 30s default it is overriding. This file scrubs every inherited AIPERF_* except AIPERF_BIN, so an operator who exported it upstream had no effect and the client kept the stock bound. Exported after the scrub like every other AIPERF_ setting here, tunable through AGENTX_HTTP_TCP_USER_TIMEOUT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI E2E report — ❌ Timeout
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_grid_runner.py) so a canonical warmup does not get killed by a synthetic-sized cap (1800s/2400s/7800s).timeout_secparam is produced (request_handlers.py), without disturbing the existing "explicit param outranks AgentX derivation" behavior.AGENTX_BASELINE_OVERHEAD_SECis left at its default (baseline.py): the default (7200s) is calibrated on GLM-5.2/Qwen3.8 measurements; a raw aiperf run against Kimi-K3 at concurrency=64 measured warmup alone taking ~12075s, already past the whole default cap. This does not change any timeout value or kill behavior — it only makes a silent multi-hour-then-killed failure show up in logs at launch time instead.Scope / known limitations
HYPERLOOM_AGENTX=1path; the default (synthetic) execution path is unchanged (verified no-op for AgentX-off in all three call sites).main(GLM-5.2 / Qwen3.8 already clear baseline/integrate_patch cleanly at canonical settings with these fixes).--benchmark-duration), a separate, unvalidated problem intentionally left out of this PR rather than guessed at.Test plan
pytest src/hyperloom/inference_optimizer/tests/test_agentx_variant_timeout.py src/hyperloom/inference_optimizer/tests/test_agentx_rebaseline_timeout.py src/hyperloom/inference_optimizer/tests/test_agentx_baseline_timeout.py src/hyperloom/inference_optimizer/tests/test_aiperf_client_sh.py -q— 26 passed, 37 skipped (require aiperf binary), 0 failed