runner: this repo now owns benchmark operations — port the campaign runner out of stellar-rpc - #16
Closed
marwen-abid wants to merge 17 commits into
Closed
marwen-abid wants to merge 17 commits into
marwen-abid wants to merge 17 commits into
Conversation
…-rpc#892) This repo now owns the full benchmark lifecycle: campaign config → run → publish to GCS → converter → viewer. The runner treats stellar-rpc as a black box it clones, builds, and drives, so it never belonged inside that repo; stellar-rpc keeps only the bench-ingest/bench-query subcommands that must link production code. Ported from stellar-rpc's bench-devbox-campaigns branch (scripts/bench-campaigns/) with the checkout-independence adaptations: - New validated config key REPO (git URL or absolute local path, default https://github.com/stellar/stellar-rpc.git). campaign.sh maintains a persistent build clone at $BENCH_ROOT/src: clone once, re-point origin at $REPO, fetch branches+tags each campaign, reset --hard + clean -fd (no -x, build caches survive), checkout --detach the resolved commit. - REF resolves inside the src clone after the fetch (default feature/full-history); the operator-checkout semantics (dirty-tree check, rev-parse-HEAD default) are gone. Benchmarking local WIP = pointing REPO at a local checkout; only committed state is benchmarkable. - bootstrap.sh keeps its no-build principle but no longer needs a standalone ~/stellar-rpc checkout: it seeds $BENCH_ROOT/src and runs the native-lib install scripts from there. - New shellcheck CI workflow gating runner/**. - runner/README.md documents the campaign bundle layout as a cross-repo contract (metadata.json from campaign.sh, invocation.json from stellar-rpc's bench subcommands) and the compatibility floor: a stellar-rpc ref that writes invocation.json (bench-run-metadata until it merges into feature/full-history). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
There was a problem hiding this comment.
Pull request overview
This PR ports the benchmark campaign runner/operations scripts into stellar-rpc-benchmarks, making this repo responsible for the full operational loop (campaign config → run → publish → convert → docs viewer) while treating stellar-rpc as a build-and-run black box.
Changes:
- Adds
runner/scripts (bootstrap.sh,campaign.sh,publish.sh) plus an example config and a runner-specific README describing the cross-repo bundle contract. - Updates docs (
README.md,SCHEMA.md) to referencerunner/as the producer for the “campaign” input layout and to document compatibility expectations. - Adds a GitHub Actions workflow to
shellcheck/bash -nlint runner scripts/configs on PRs touchingrunner/.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
SCHEMA.md |
Updates campaign producer reference to runner/campaign.sh and runner/README.md. |
runner/README.md |
Documents runner purpose, $BENCH_ROOT layout, bundle contract, and compatibility floor. |
runner/publish.sh |
Adds an idempotent publisher with immutability guard for gs:// and s3://. |
runner/example-campaign.cfg |
Adds an annotated example campaign config for operators. |
runner/campaign.sh |
Adds the main config-driven runner: clone/fetch/reset, build, dataset prep, bench loops, manifests, optional publish. |
runner/bootstrap.sh |
Adds an idempotent devbox bootstrap for NVMe + toolchain + native libs + env setup. |
README.md |
Adds a “Run a campaign” section and updates repo layout to include runner/. |
.github/workflows/shellcheck.yml |
Adds CI linting (bash -n + shellcheck) for runner scripts/configs. |
Comments suppressed due to low confidence (2)
runner/campaign.sh:318
- Same issue as packs-gs: if $root exists but is empty/non-golden,
mv "$root.partial" "$root"will produce an incorrect nested directory. Remove any pre-existing $root before backfill.
run rm -rf "$root.partial"
runner/campaign.sh:342
- Same issue as packs-gs: if $root exists but is empty/non-golden,
mv "$root.partial" "$root"will produce an incorrect nested directory. Remove any pre-existing $root before fixture generation.
run rm -rf "$BENCH_ROOT/fixture/$name" "$root.partial"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+137
to
+140
| if [ "$_in_array" -eq 1 ]; then | ||
| case "$_stripped" in *')'*) _in_array=0 ;; esac | ||
| continue | ||
| fi |
Comment on lines
+309
to
+311
| run mkdir -p "$root.partial" | ||
| run gcloud storage rsync -r "$loc" "$root.partial" | ||
| run mv "$root.partial" "$root" |
Comment on lines
+251
to
+254
| ensure_src() { | ||
| if [ ! -d "$SRC/.git" ]; then | ||
| run git clone "$REPO" "$SRC" | ||
| fi |
Comment on lines
+102
to
+103
| if ! grep -q '# bench-campaigns env' "$HOME/.bashrc"; then | ||
| cat >> "$HOME/.bashrc" <<'EOF' |
…olden mv CI's ubuntu-24.04 image ships ShellCheck 0.9.0, which flags the `[[ ]] && [ ] || die` validation line as SC2015 (0.11.0 knows die never returns and stays quiet — why this passed locally). Brace-group the two tests so the construct is explicit; behavior is unchanged. Also address a Copilot review finding: prepare_dataset's stage-then-rename (`mv $root.partial $root`) nests the partial inside $root when a bare empty $root survives (e.g. `rm -rf .../golden/<name>/*` instead of removing the dir). golden_present then reports the poisoned root as present, so every re-run skips the fetch and dies at the ledgers post-check. Clear $root — provably absent or empty on this branch — before staging in all three remote kinds; packs-gs keeps $root.partial so an interrupted rsync still resumes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* runner: add --resume, crash-safe metadata, and campaign.log A campaign is hours of work — the phase-1 reference run took ~17 hours, with single hot-ingest legs near 5.5 — and today a crash at the last rep costs all of it: re-running mints a new $RES with a fresh stamp and redoes every finished leg. A killed campaign also leaves no metadata.json (written only at the end), so its partial bundle is not even parseable for salvage, and nothing the runner printed survives outside the operator's terminal. Three additive changes: --resume <results-dir> continues into an existing bundle instead of starting a new one. The directory's basename must parse as <NAME>-<sha>-<stamp> with NAME from this config and sha equal to what REF resolves to now (resuming onto a different commit would mix two binaries inside one bundle), and it must be this BENCH_ROOT's results directory. Its stamp is reused, so the run id — and every identity derived from it — is unchanged. A timed leg is skipped when its --out dir already holds both invocation.json and driver.csv; the bench subcommands write invocation.json as the run completes, next to the driver.csv they stream during it, so a dir with one but not the other was mid-flight when the campaign died and is wiped and re-run. Golden dataset prep and the binary build already self-skip. Since a resume can skip every hot-ingest leg of a cell, query-hot first checks that the hot DB those legs leave behind is still there. metadata.json is now written as soon as $RES exists, without finished_at, and rewritten with it at the end: a killed campaign leaves a parseable bundle, and --resume recovers the original started_at from it so the manifest still spans the whole campaign. A resumed bundle carries campaign.resumed; the converter passes unknown campaign keys through to campaign.config and reads only started_at, so both are additive to the contract. $RES/campaign.log captures the runner's stdout and stderr from the moment the bundle directory exists, appended so a resumed campaign's sessions accumulate in one file with a header line per session. Plain --dry-run prints exactly the plan it printed before; --dry-run --resume inspects a real bundle and prints, per leg, the skip note or the command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * runner: make the dry-run placeholder sha hex so --dry-run --resume works --resume matches the bundle basename against <NAME>-<sha>-<stamp> with sha as 8 hex digits. Without a clone, --dry-run plans with the placeholder 'drysha00', which is not hex — so `--dry-run --resume` on any machine that has not cloned stellar-rpc rejected its own run ids as malformed: error: --resume: 'example-drysha00-...' is not a <NAME>-<sha>-<stamp> results directory ...naming the one thing that was not actually wrong. On the devbox REF resolves and the real sha is used, so this only bit the everywhere-dry-runnable path. Use 'deadbeef' — still obviously a placeholder, and hex. The three resume guard rails now each report their own reason: a well-formed id with a different sha gets the mixed-binaries error, a different NAME gets the wrong-campaign error, and only a genuinely malformed name gets the malformed-name error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Three ways to get a run into this repo had accumulated, and they disagreed: the README taught hand-run gcloud + six-variable `make convert` + a hand-written commit; scripts/ingest.sh already automated all of it and the README never mentioned it; ingest.yml duplicated the converter call inline, asked for five inputs metadata.json already carries, and pushed straight to main. scripts/ingest.sh is now the single path, unchanged in behavior: - ingest.yml delegates to it in full mode. Inputs drop from six to three (gcs_path, dataset_kind, optional extra_args forwarded to convert.py after `--`); the inline gcloud download and converter invocation are gone, because the script does both. Runs now arrive as a reviewable run/<run_id> PR that pr-preview.yml renders, instead of a commit on main. Permissions gain pull-requests: write, and the step sets GH_TOKEN so gh works headlessly. The WIF guard and the two google-github-actions steps are untouched — the GCP side is still pending and that error text is deliberate. - README's ingestion section leads with `make ingest`, documents what comes from metadata.json, the three modes, the run/<run_id> PR, and the --force rail. `make convert` stays, repositioned as the layer underneath for legacy bundles that predate metadata.json. The GitHub Action section describes the delegated workflow and keeps the pending-WIF caveat. - Repo layout tree brought in line with git ls-files: scripts/ingest.sh, all four workflows, and the docs/ pages and data files that were missing. ingest.sh itself changes only in its CI NOTE (which now states what ingest.yml does rather than what it is expected to do) and the usage() sed range that tracks the header's length. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Failed runs now write invocation.json too (with an error field), and the producer's keys are camelCase. Converter normalizes both spellings and warns loudly on error-bearing invocations; resume_skip treats an error-bearing manifest as a failed leg; fixtures write the #907 shape; SCHEMA.md and runner/README.md document the merged compatibility floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Go module at runner/ (module github.com/stellar/stellar-rpc-benchmarks/runner,
BurntSushi/toml pre-declared as the one allowed dependency). cmd/campaign
dispatches run/plan/preflight/publish through per-subcommand flag.FlagSets —
stubs that print their intended usage and exit 2 — via a parseArgs helper
that accepts flags and positionals in any order and honors the -- terminator.
internal/{config,plan,run} pin the package layout for tasks 2-4. Make targets
runner-build/runner-test and the runner-go.yml workflow gate go vet + go test
in CI; shellcheck.yml stays until the bash runner is deleted (task 12).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
internal/config replaces the sourced-bash config: Load decodes TOML over pre-filled defaults, rejects unknown keys via MetaData.Undecoded (the check the bash pre-scan could not safely provide), and ports every validation rule from campaign.sh with the same operator-facing specificity. Fixture datasets get a dedicated ledgers key (*int, 0 = whole chunk) instead of overloading location; the metadata.json quirks the converter depends on — query as "yes"/"no", query_concurrency as a comma string, fixture location recorded as the decimal ledger count — live in one exported mapping layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…test internal/plan turns a validated config into the campaign as data: an ordered []Step with explicit needs, argv copied flag-for-flag from the bash suite loops (all ingest-cold, then ingest-hot, query-cold, query-hot), the bsb-s3 AWS_EC2_METADATA_DISABLED env quirk, and the query-hot → last-hot-rep dependency (each rep rewrites the hot DB, so only the final rep guarantees a whole one). plan.json is schema_version 1; Print renders the bash dry-run format. campaign plan <cfg> works offline on any machine, with a placeholder sha when the ref is not locally resolvable. Golden test pins the full plan; config now also rejects duplicate chunk IDs, which the step-ID space requires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e, keep-going internal/run walks the plan sequentially and owns the completion marker the bash runner borrowed from stellar-rpc: every timed leg writes leg.json (schema_version 1) into its --out dir, success or failure, so resume no longer infers completion from another process's write ordering. Resume trusts leg.json first and falls back to the bash-era heuristic (invocation.json without error + driver.csv) for old bundles; anything ambiguous — partial dirs, corrupt sentinels, dangling symlinks — wipes and re-runs. Failures keep the campaign going: dependents are skipped transitively, independent legs still run, and the summary + nonzero exit report what happened (--fail-fast opts out). AcquireLock flocks $BENCH_ROOT/.campaign.lock so two campaigns cannot share a build clone. Dataset prep and publish steps fail honestly until tasks 8 and 9 port them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
internal/preflight derives its checks from the config: git/make always, go+cargo only when the ref will actually be built (cargo also accepted at ~/.cargo/bin, where rustup puts it), gcloud only for packs-gs datasets or a gs:// publish_uri, aws only for s3:// publishing (bsb-s3 needs neither — the bench binary's SDK reads the public bucket). The publish root must be listable with current credentials before the campaign starts; ListRoot is stricter than publish.sh's uniform empty-vs-error case statement — gcloud is "empty" only on its documented "matched no objects" message, aws only when totally silent — because a false empty would pass the exact credential check preflight exists to make. Free disk below 100 GiB warns; the /mnt/nvme mountpoint check is ported verbatim. campaign preflight <cfg> is wired; run picks it up in task 8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ty, status internal/bundle owns the bundle-root manifest: ReadMetadata, the additive status vocabulary (running/finished/failed — the writer lands with task 7), and ValidateResume, which replaces bash's basename parsing with identity from metadata.json and adds the check bash never had: the stored config copy must be byte-identical to the config being resumed, or the runner prints a unified diff and refuses. This closes the review's only silent-data-corruption finding — bash overwrote the stored copy on resume, so edited knobs produced mixed data under a manifest that uniformly claimed the new knobs. Also kept: the name, built-commit, and BENCH_ROOT checks with bash's messages, started_at recovery, and a basename guard so a hand-edited config_file cannot point the comparison outside the bundle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne-metadata.txt encoding/json replaces ~140 lines of bash-driving-jq with exact field parity: query stays the string "yes"/"no", query_concurrency stays a comma string, fixture datasets record their ledger count in location, empty hardware facts are omitted rather than "", finished_at appears only on the final write, and campaign.resumed only on resumed bundles — all pinned by a golden test plus explicit quirk assertions. The additive status field (running/finished/failed) rides along. IMDSv2 is queried once and shared between metadata.json and machine-metadata.txt (bash asked twice), with 2s timeouts and clean absence off EC2. The fsync probe is native Go (O_SYNC, 4 KiB x 2000 — dd's oflag=dsync does not exist on macOS) and says so in its own output line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n wiring EnsureSrc/ResolveRef port the build-clone convergence (clone once; re-point, fetch, hard-reset, clean -fd — deliberately no -x, build caches survive; remote-tracking refs first so a stale local ref never shadows the fetched tip). Dataset preparation lands for all four kinds with the .partial→rename convention: packs-gs keeps a half-fetched .partial for rsync to resume into, bsb-s3 and fixture wipe theirs, and every wipe is plan data (step.pre_clean) so the dry-run prints the full destructive choreography exactly as bash did. campaign run is wired end to end: BENCH_ROOT lock, preflight, resume validation, config copied only on fresh sessions (the stored copy is finding 1's ground truth), campaign.log tee, up-front metadata + plan.json, the executor walk (binary.txt written right after a successful build), and an epilogue — machine metadata, final metadata with status, tar — that runs even when legs failed, because that is exactly when the bundle matters. Ingest-cold scratch is now also deleted after each rep (deliberate change; nothing reads it). Publish prints a manual-command note until task 9. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…epilogue internal/publish ports publish.sh with exit-code-aware process handling: run_id is the bundle basename, the destination is <root>/<run_id>/, gs:// syncs with gcloud storage rsync -r and s3:// with aws s3 sync, and the immutability check distinguishes an empty prefix from a real listing error per tool (a false "empty" would bypass immutability, so gcloud must say "matched no objects" and aws must be totally silent). --force is documented as a merge, not a replace. preflight.ListRoot now delegates to publish.List — one listing implementation for both callers. campaign publish is wired (dest-root defaults to $PUBLISH_URI), and campaign run's epilogue publishes after the campaign-done note, gated on the tar succeeding, exiting 1 with the exact retry command on failure — never corrupting the done signal. The success line stays `published: <dest>`, machine-greppable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A committed POSIX-sh stub stands in for stellar-rpc: it accepts the real bench subcommand flags, writes fixtures.py-shaped driver.csv/hot.csv and a #907 camelCase invocation.json (with an error field on demand), and a control file makes any leg fail or hang. Fake git/make on PATH stand in for the toolchain, so the whole campaign — build, fixture prep, eight timed legs, provenance, tar — runs for real against a temp BENCH_ROOT. Scenarios: happy path, SIGKILL mid-leg then --resume (finished legs skipped, the partial leg wiped and re-run, sessions accumulating in one log), a failed last hot rep under keep-going (query-hot skipped, status failed, exit nonzero), an edited-config resume refused with the stored copy intact, a tar failure gating publish, and — the strongest contract test this pipeline has — the real converter/convert.py converting the produced bundle cleanly. Test subprocesses are deadline-bounded and killed/reaped exactly once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compiler versions move benchmark numbers; two boxes bootstrapped months apart must build with the same compilers. GOVER=go1.26.5 (bump together with runner/go.mod's go directive) and RUSTVER=1.92.0, each with a re-baseline note. The pins are effective, not defaults: an off-version /usr/local/go is replaced with the pinned one, and an existing rustc that reports any other version is re-pinned through rustup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gain runner/campaign.sh, publish.sh, and example-campaign.cfg are gone; the Go CLI they were ported into is the runner. example-campaign.toml carries the same annotation quality; runner/README.md is rewritten around the CLI — the authoritative TOML key reference (moved from the deleted script header), a .cfg → .toml migration table for existing devbox configs, the new resume semantics (metadata identity, config-diff guard, the leg.json sentinel with the bash-era fallback), and the bundle contract additions (plan.json, leg.json, metadata.status). The top-level README's operator flow uses the new commands and documents that retrying a partial upload needs --force (a merge, not a replace). SCHEMA.md § Inputs gains the additive files; shellcheck.yml rescopes to bootstrap.sh, scripts/, and the e2e stub. Review hardening: a leg.json only counts as complete when it carries schema_version 1, the leg's own id, and an explicit exit_code — a sentinel that cannot prove it is this leg's success record classifies as partial and re-runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
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.
Why
This repo becomes the home of benchmark operations, closing its pipeline into a full loop: campaign config → run → publish to GCS →
converter/convert.py→docs/viewer. The campaign runner treats stellar-rpc as a black box — it clones it, builds the configured ref, and drives thebench-ingest/bench-querysubcommands — so it never belonged inside stellar-rpc. The split: stellar-rpc keeps only what must link production code (the bench subcommands); the runner lives here.This ports and supersedes the scripts portion of stellar/stellar-rpc#892 (
bench-devbox-campaigns), which will be closed unmerged.Example config
$BENCH_ROOTlayout (default/mnt/nvme/bench)What changed
runner/—bootstrap.sh,campaign.sh,publish.sh,example-campaign.cfg, ported from stellar-rpc with the checkout-independence rework:REPO(git URL or absolute local path; defaulthttps://github.com/stellar/stellar-rpc.git).campaign.shmaintains the persistent build clone at$BENCH_ROOT/srcfrom it. Benchmarking local WIP = pointingREPOat a local checkout; only committed state is benchmarkable.REF(defaultfeature/full-history) now resolves inside the src clone after fetching$REPO's branches + tags; the operator-checkout semantics (dirty-work-tree check,rev-parse HEADdefault) are removed.bootstrap.shstill only provisions (no builds), but needs no standalone~/stellar-rpccheckout: it seeds$BENCH_ROOT/srcand runs the native-lib install scripts from there.runner/README.md— the campaign bundle layout as a cross-repo contract (metadata.jsonwritten bycampaign.shhere;invocation.jsonwritten by stellar-rpc's bench subcommands), and the compatibility floor: the runner requires a stellar-rpc ref whose bench subcommands writeinvocation.json— thebench-run-metadatabranch or a descendant (its merge intofeature/full-historyonce landed)..github/workflows/shellcheck.yml— lintsrunner/*.sh+runner/*.cfg(shellcheck +bash -n) on PRs touchingrunner/.runner/campaign.sh/runner/README.mdfor the producer side.Non-goals honored: no Go port (
benchctlremains a possible future step), no converter/viewer changes, no results-schema changes (metadata.jsonshape is byte-compatible;REPOprovenance is recorded inbinary.txt/machine-metadata.txtonly).Verification
shellcheckclean andbash -nclean on all three scripts + the example cfg (same checks the new CI workflow runs).BENCH_ROOT=<tmpdir> runner/campaign.sh runner/example-campaign.cfg --dry-runon macOS prints the full command plan; the clone/fetch/reset/checkout sequence references only$BENCH_ROOT/srcand the defaultREPOURL — no path inside any stellar-rpc checkout. Also exercised with a local-pathREPO+ real ref resolution (bench-run-metadata→b80e2695in all derived paths), and the config validator rejecting relative/non-gitREPOvalues and unknown keys.make test/make smoke: unchanged vsmain— both have pre-existing failures on a cleanmainworktree (golden tests error insetUpClassfrom local dataset state; the same 3 smoke assertions fail identically), and this PR touches neither the converter nor the viewer.🤖 Generated with Claude Code