bench-campaigns: config-driven campaign runner for ingest benchmarks - #892
bench-campaigns: config-driven campaign runner for ingest benchmarks#892marwen-abid wants to merge 8 commits into
Conversation
Build on the bench-ingest harness (#771) with the tooling needed to run whole benchmark campaigns on a dedicated devbox: - scripts/bench-devbox/campaign.sh runs a campaign from a config file: it builds the requested ref, materializes the configured datasets (local or GCS ledger packs), runs the cold/hot ingest legs with the configured repetitions and pacing, and collects results under a git-describe + timestamp key with a metadata.json manifest recording the config, datasets, hardware, and timings. - scripts/bench-devbox/bootstrap.sh prepares a fresh or restarted box: mounts the NVMe instance store, installs the toolchain, builds, and verifies the bench package. - scripts/bench-devbox/publish.sh uploads finished campaign bundles to object storage. - scripts/bench-devbox/example-campaign.cfg documents every config key; real campaign configs live outside the repo. bench-ingest itself now writes a per-invocation invocation.json (binary version, commit, flags, start/end times) so every result directory is self-describing, and pace_lag rows include on-time ledgers instead of dropping zero-lag samples, so the percentiles reflect the whole paced run. campaign.sh can also drive query benchmarks and fixture generation; those subcommands arrive in follow-up PRs, so configs that use them (QUERY=yes, fixture datasets) fail at runtime until then.
7f1449e to
c48a55c
Compare
There was a problem hiding this comment.
Pull request overview
Adds reproducible, config-driven ingest benchmark campaigns for dedicated devboxes.
Changes:
- Adds campaign orchestration, bootstrap, publishing, and example configuration.
- Records per-invocation provenance for cold/hot ingest benchmarks.
- Includes zero-lag samples in paced-run statistics.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/bench-devbox/campaign.sh |
Orchestrates campaign builds, datasets, runs, and metadata. |
scripts/bench-devbox/bootstrap.sh |
Prepares benchmark devboxes. |
scripts/bench-devbox/publish.sh |
Publishes campaign results. |
scripts/bench-devbox/example-campaign.cfg |
Documents campaign configuration. |
bench/invocation.go |
Generates invocation manifests. |
bench/invocation_test.go |
Tests manifest generation. |
bench/cold.go |
Records cold-run provenance. |
bench/hot.go |
Records hot-run provenance. |
bench/command.go |
Passes Cobra commands into runners. |
bench/bench_test.go |
Updates runner tests. |
bench/csvsink.go |
Includes zero pace lag values. |
bench/csvsink_test.go |
Tests zero/mixed lag aggregation. |
Comments suppressed due to low confidence (1)
scripts/bench-devbox/campaign.sh:247
- The REF build also bypasses the Makefile's injected version/commit/build metadata, leaving every generated
invocation.jsonwith default binary identity fields. Use the existing identity-aware build target here as well.
run make build-libs
run go build -o "$BIN" ./cmd/stellar-rpc
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The base branch renamed cmd/stellar-rpc/internal/fullhistory to internal/rpcv2 and split the binary into ./cmd/stellar-rpc/rpcv1 and ./cmd/stellar-rpc/rpcv2. All conflicts were rename fallout in the bench package: take the base's rpcv2 import paths and keep this branch's invocation.json writer plus the *cobra.Command threading through runCold/runHot. Also repointed the parts a textual merge cannot see: - bench/invocation.go now reads the build metadata from internal/version, where those globals moved from internal/config. - scripts/bench-devbox/bootstrap.sh verifies the rpcv2/bench package and checks the v2 binary that `make install` now produces alongside v1. - scripts/bench-devbox/campaign.sh builds ./cmd/stellar-rpc/rpcv2, since ./cmd/stellar-rpc is no longer a main package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
errcheck: check the pflag Set error in TestCaptureFlags; gosec: write invocation.json 0600; testifylint: assert.Equal for the typed int64 comparison. The snake_case JSON tags stay (they match the metadata.json schema campaign.sh writes) with per-field tagliatelle suppressions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- validate the config file before sourcing it, so a config can only assign the documented keys and cannot clobber the runner's variables - build through make build-rpc-v2 so the binary carries the GOLDFLAGS version/commit/branch metadata that invocation.json reports - refuse untracked files too in the REF-build clean-tree check - scope AWS_EC2_METADATA_DISABLED to the public-bucket backfill command instead of exporting it globally, which broke IAM-role publishing - assemble metadata.json with jq instead of a heredoc so special characters in values cannot produce invalid JSON - distinguish an empty publish destination from a failed listing and abort on the latter - warn in bootstrap when gcloud/aws CLIs are missing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
scripts/bench-devbox/campaign.sh:232
- When no
REFis supplied, this provenance can identify an altered binary as a clean commit:git describe --dirtyignores untracked files, even though untracked Go files are compiled (as the later comment notes). Refuse a dirty/untracked checkout here, or bundle/hash all working-tree changes soSHA,built_commit, and the retained metadata can identify the actual source.
BUILT_COMMIT=$(git rev-parse HEAD)
SHA=$(git describe --always --dirty --abbrev=8)
scripts/bench-devbox/campaign.sh:603
NAMEmay start with-, so a valid value such as--helpproduces a member name thattarparses as an option and the campaign fails only after all benchmark work finishes. Terminate option parsing before the generated member name.
tar -C "$BENCH/results" -czf "$TARBALL" "$NAME-$SHA-$STAMP"
scripts/bench-devbox/campaign.sh:611
- The advertised retry command is not executable from the campaign's current directory: the script has
cd'd to the repository root, whilepublish.shis underscripts/bench-devboxand is not necessarily onPATH. Printing the absolute script path and shell-escaping each argument makes this the exact copyable retry command promised by the message.
note "publish failed — data is safe in $RES and $TARBALL; retry with: publish.sh $RES $PUBLISH_URI"
cmd/stellar-rpc/internal/rpcv2/bench/cold.go:158
- Failed backfills can write partial CSVs at lines 146-148, but they return before this success-only invocation write. Those partial result directories therefore remain non-self-describing. Record invocation metadata from a finalization path for both success and failure, ideally including the completion status/error.
if err := writeInvocationJSON(opts.OutDir, cmd, captureFlags(cmd), startedAt, time.Now().UTC()); err != nil {
cmd/stellar-rpc/internal/rpcv2/bench/hot.go:166
- Failed or incomplete hot runs write partial CSVs at lines 156-158 and return before this success-only invocation write, so those output directories still lack the provenance this change is intended to guarantee. Finalize invocation metadata on both paths and distinguish failed/incomplete runs in the record.
if err := writeInvocationJSON(opts.OutDir, cmd, captureFlags(cmd), startedAt, time.Now().UTC()); err != nil {
scripts/bench-devbox/bootstrap.sh:39
- If
/mnt/nvmeis already a mountpoint, the script never verifies that it is backed byNVME_DEV. It can therefore silently run benchmarks on an EBS or other filesystem, invalidating the storage/fsync measurements despite having validated a different instance-store device. Check the existing mount source (for example withfindmnt) and fail unless it resolves toNVME_DEV.
if ! mountpoint -q "$MOUNT"; then
sudo mkdir -p "$MOUNT"
sudo mount -o noatime "$NVME_DEV" "$MOUNT"
sudo chown "$USER" "$MOUNT"
fi
scripts/bench-devbox/campaign.sh:132
- This prefix check does not make the sourced config assignment-only. For example,
NAME=ok; REPO=/tmppasses and then overwrites an internal variable, whileNAME=$(some-command)executes a command duringsource. As a result, unknown assignments and commands are not actually rejected. Parse the config as data, or validate the complete Bash syntax and reject separators/substitutions before sourcing it.
This issue also appears on line 231 of the same file.
[[ $_stripped =~ $_re_cfg_key ]] ||
The campaign is the central concept; bootstrapping a machine is just a prerequisite. Also replaces "devbox" wording with generic language, since these scripts work on any Linux benchmark machine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Failed or interrupted runs keep their partial CSVs but had no invocation.json, so exactly the costly interrupted runs lost their binary, flag, and timing identity. Record the run error in an optional `error` field and write the record on the failure paths of both cold and hot runs; success records are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
campaign.sh now owns building the binary under test in a dedicated clone ($BENCH_ROOT/src) cloned from the operator's checkout, so unpushed local refs resolve, the operator's checkout is never modified, and the old checkout/restore dance (EXIT trap, dirty-tree refusal for REF) is gone. REF unset now means the checkout's HEAD commit; uncommitted changes are a hard error since they can't be part of the built binary. The clone is hard-reset per campaign but keeps gitignored build caches, so rebuilding a nearby commit is incremental. bootstrap.sh no longer builds or tests — it only bootstraps (NVMe, toolchain, native libs, clone, env); campaign.sh builds on first run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Why
We run ingest benchmark campaigns on-demand. One campaign is dozens of
bench-ingestinvocations — every (dataset, chunk) cell, cold and hot, repeated N times — plus dataset preparation and result collection. Done by hand this is slow and error-prone, and the results weren't self-describing enough to compare across machines and dates.This PR turns a campaign into a single config file and one command.
How it works
A campaign is a small bash config:
Then, on the benchmark machine:
campaign.shwalks the config top to bottom:REF(default: your checkout's HEAD) is built in a persistent clone at$BENCH_ROOT/srcinto a versioned binary ($BENCH_ROOT/bin/stellar-rpc-<sha>)DATASETSare materialized into$BENCH_ROOT/golden/<name>/:packs-localis used in place,packs-gsis fetched once from GCS (staged atomically, so a failed fetch never looks like a complete dataset) and reused across runs.INGEST/RUNS/CLOSE_INTERVALdrive the benchmark loop: for each (dataset, chunk) cell,RUNSfreshbench-ingest cold/hotprocesses, each with its own--outdirectory. Each run writes aninvocation.json(binary version, commit, flags, timings, error if it failed) next to its CSVs — this is the Go-side change that makes every result directory self-describing.$BENCH_ROOT/results/<NAME>-<sha>-<stamp>/with ametadata.jsonmanifest (config, datasets, hardware, hostname, timings) and are bundled (tar); withPUBLISH_URIset,publish.shuploads the bundle to object storage (immutable — it refuses to overwrite an existing run).After a run,
$BENCH_ROOTlooks like:The results directory is also tarred to
/tmp/bench-results-<NAME>-<sha>-<stamp>.tgz(the EBS root, so the bundle survives an instance stop).--dry-runprints every command the campaign would execute without building, downloading, or benchmarking anything.What changed
scripts/bench-campaigns/— new:campaign.sh(the runner, which also owns building the binary under test),bootstrap.sh(idempotent devbox prep after a stop/start: mounts the NVMe, installs the toolchain and native libs — no builds),publish.sh(bundle upload), and a commentedexample-campaign.cfg.bench-ingest— each cold/hot run writesinvocation.jsonalongside its CSVs, on failure too.pace_lagfix — zero-lag (on-time) ledgers are now included in thepace_lagpercentiles, so they describe the whole paced run rather than only the late ledgers.Notes for reviewers
campaign.shalso knows how to drive query benchmarks (QUERY=yes) andfixturedatasets; those subcommands land in follow-up PRs. Until then, configs using them fail at runtime with "unknown command" — pack-based ingest campaigns don't use either.BENCH_ROOT(the working root) stays an environment variable; everything else is validated config keys.Closes #896
🤖 Generated with Claude Code