Claude/helmsdeep performance runner y0q51y - #51
Merged
Merged
Conversation
The harness's own locust runner drove a flat ramp of one fixed query and reported blended request stats. It could say how long a submit took; it could not say what concurrency the service actually sustains, which is the question a performance test exists to answer. HelmsDeep answers it. It owns the per-layer step-load ramps, the TRAPI corpuses, the ARS submit/poll/merge protocol, and the knee/checkpoint arithmetic, so `performance_test_runner.py` is now a driver: pick the run type, run the CLI, read what it wrote. Two things move out of the harness on purpose, and the module docstring says so: * The ramp. HelmsDeep's LoadTestShape drives users, spawn rate and duration from its own stages table, so a test case's `test_run_time`/`spawn_rate` are unused and a run takes as long as the ramp says. HelmsDeep can compress a run to a wall clock, but its own docs are explicit that a compressed run is indicative rather than a measurement, so the harness never asks for it. * The queries. HelmsDeep sends its own corpus, varying the pinned entity per request. The asset's TRAPI query is not what goes under load, so the radiator log now carries the run plan (layer, ramp, SLO, checkpoints) instead of a query that misrepresents the run. Run type: `ars` maps to the async targets, every other component to the sync ARA ones. The profile within that layer defaults to the layer's single-class corpus (knee only) and is switched to `mixed` -- the 2:1 inferred/Pathfinder acceptance profile, the only one carrying pass/fail checkpoints -- or `pathfinder` by `--performance_profile` or a test case's `test_runner_settings`. Reporting: HelmsDeep's `summary.json` and `report.html` are uploaded to Slack verbatim, each with the run's checkpoint verdict as its comment, and the Slack summary leads with the overall pass/fail before the per-checkpoint lines and the knee. `summary.json` goes up as HelmsDeep wrote it rather than being reformatted, so what Slack claims and what the file says cannot drift. Three ways this refuses to overstate a result: * A run with no checkpoints reports "no checkpoints configured", not a pass. A knee-finding run never tested a pass/fail criterion. * A run that produced no summary is a failure, not an absence of failures. * The exit code is recorded but is never the verdict. HelmsDeep exits 1 on a missed checkpoint, but locust beneath it exits 1 whenever any single request failed -- a run with every checkpoint met and one 500 along the way still exits 1. The verdicts come from the summary. A missed checkpoint or a run that never completed is also FAILED in the Information Radiator, so the dashboard agrees with Slack. `perf_plots.py` rendered charts from locust's `stats_history`, which no longer exists here; HelmsDeep's HTML report carries the charts. It and matplotlib are removed rather than left as a module that cannot run. Verified end to end against a stub ARA on both paths: a healthy service (three checkpoints PASS, knee reported, both artifacts uploaded) and one returning 500s (three checkpoints FAIL, no stage meeting the SLO, artifacts still uploaded carrying the FAIL). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uu7mLrTmyXzLSY8kyR3tjD
`download <suite>` refetches the whole NCATSTranslator/Tests zip every run, so
changing one CURIE to see what a service does with it meant either editing a
fork of that repo or hand-assembling JSON for `run`. Ship a few small suites and
a subcommand that reads them off disk:
test-harness --local load local_acceptance
Three suites, one per harness path, kept small enough to read in one screen:
`local_acceptance` (an MVP1 treats case and an MVP2 affects case, so
`--query_type` has something to filter), `local_performance` (a
QuantitativeTest, run by HelmsDeep), and `local_pathfinder`. They are ordinary
Translator Testing Model `TestSuite` documents, structurally identical to the
downloaded ones and named the same way -- the suite name is the file's basename
either way -- so a suite that works here works upstream.
Lookup order: an explicit `--tests_dir`, then `test_suites/` in the working
directory when it exists, then the copy shipped with the package. The middle
step is what lets a checkout's own edits win without a flag; the last is why
`package_data` carries the suites, so this works from a non-editable install
too and not only from a clone.
The failures here are ones a person hits while editing JSON by hand, so they
say what to do rather than surfacing as an empty run: a typo'd suite name lists
the suites that do exist, a missing directory says so, and a suite that fails
validation reports the parse error instead of quietly loading zero tests.
`test_harness/test_suites/README.md` covers which fields actually drive
behavior -- notably that a performance case's `test_run_time`, `spawn_rate` and
asset query are inert, since HelmsDeep owns the ramp and the corpus, and that
`test_runner_settings` is what picks its profile.
Also corrects `run`'s help, which described its argument as a path to a file;
it takes the JSON itself, and `load` is now the way to run tests from a file.
Verified by running both `load local_acceptance --query_type MVP1` and
`load local_performance --performance_profile mixed` end to end against a stub
service, and by confirming the suites ship inside a built wheel.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu7mLrTmyXzLSY8kyR3tjD
HelmsDeep loads exactly one layer per run and the harness resolves one target
per invocation, so characterizing the ARS and each ARA takes four runs. They
must not overlap -- the stack cascades ARS -> ARAs -> KPs, so loading two layers
at once double-loads whatever sits underneath and corrupts both measurements --
which rules out four parallel Jobs. `test-harness-sweep` runs them in order,
waiting for each, so one scheduled Job can do the sweep:
test-harness-sweep --suite performance_tests --download \
ars=https://ars.ci.transltr.io aragorn=... arax=... bte=... \
-- --performance_profile mixed
Each NAME=URL pair becomes one `test-harness --target_url URL --target NAME`
invocation; anything after `--` is passed through to all of them. The target
list can come from PERFORMANCE_TARGETS instead, which keeps "which services do
we sweep" a manifest edit rather than an image rebuild.
Two things it gets right that a shell `&&` chain would not:
* Every target runs even when an earlier one fails -- a broken ARS shouldn't
cost the ARA numbers -- and the sweep still exits non-zero afterward, with a
per-target summary of exit codes and durations.
* Each target gets its own --output_dir subdirectory. HelmsDeep names its raw
files `helmsdeep_<run_type>_case_<test id>_*`, which mentions neither the host
nor the target, so every ARA in a sweep writes identical names; sharing one
directory silently overwrote the previous target's CSVs and HTML report.
Confirmed against two stub services: both wrote
`helmsdeep_aras_mixed_case_TestCase_1_*` and only the subdirectory kept them
apart.
Runs out-of-process per target rather than calling main() in a loop, so a crash
or a leaked locust greenlet in one target can't take the rest of the sweep with
it.
deploy/cronjob.example.yaml is a worked CronJob carrying the fields a run of
this length actually needs: concurrencyPolicy Forbid (without it the next
schedule starts a second sweep against the same services -- precisely the
concurrent load the layering rule forbids), backoffLimit 0 so a failure doesn't
silently re-run hours of load and re-post every Slack message, an
activeDeadlineSeconds ceiling, PYTHONUNBUFFERED so kubectl logs isn't blank for
hours, and CPU requests == limits so a throttled load generator doesn't measure
its own scheduling delay instead of the service's latency.
Verified by running a real two-target sweep against stub services: sequential
(2.6 min then 2.5 min, 5.1 min total, no overlap), correct per-target artifacts,
and the right aggregate exit code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu7mLrTmyXzLSY8kyR3tjD
The harness returned None from every path, and cli() discarded it, so it exited 0 unconditionally. That was right for red tests and wrong for everything else: a typo'd suite name, or --target_url without --target, produced a green run that did nothing at all. On a schedule that is the failure nobody notices, because there is nothing to notice -- no Slack message, no dashboard entry, a healthy CronJob, for as long as it takes someone to wonder where the reports went. Give the exit status one meaning: did the run happen? * Exit 0 -- it happened, whatever it found. Failed acceptance tests, a missed performance checkpoint, a service that was down and produced no summary: those are results. They already reach Slack and the Information Radiator, and a slow ARA is news for the channel, not a broken cron job. * Exit 1 -- the harness could not carry out the run: half-configured target override, no subcommand, a suite that isn't there, or a --query_type that filtered everything out. So test outcomes stay invisible to the scheduler, and only operational failure is visible to it -- deliberately, and now under test in both directions rather than as an accident of returning None. The sweep needs no policy of its own: since the harness exits 0 for any test outcome, its existing "non-zero if any target's invocation failed" already means "a run couldn't be carried out". Its docstring and summary line now say that, because the semantics rest entirely on the harness's contract and were only implicit before. Verified end to end: a service failing all three checkpoints (100% errors) reports FAIL in Slack, writes checkpoints_passed false, and the sweep exits 0; a missing suite, a half-configured override, and an empty query-type filter each exit 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uu7mLrTmyXzLSY8kyR3tjD
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
2 participants
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.
No description provided.