Skip to content

feat(bench): add managed nightly RPC benchmarks - #3944

Open
danielntmd wants to merge 1 commit into
mainfrom
danielntmd/benchmark-endpoints
Open

feat(bench): add managed nightly RPC benchmarks#3944
danielntmd wants to merge 1 commit into
mainfrom
danielntmd/benchmark-endpoints

Conversation

@danielntmd

@danielntmd danielntmd commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a benchmark image containing k6, corpus-gen, and a stable nightly.sh interface for preflight, corpus generation, and benchmark execution.
  • Generates and checksum-verifies per-case corpora from bench/rpc/corpus/all.json, supporting additive case changes and reuse through persistent storage.
  • Runs sequential warmup and measured k6 benchmarks with strict failure handling and produces a compact summary.json with load, corpus, snapshot, image, and per-case performance data.
  • Adds Docker Compose support for local execution, CI publication of latest and commit-addressed images, and a static GitHub Pages results viewer.
  • Keeps deployment concerns in ArgoCD (https://github.com/NethermindEth/argocd/pull/9780), including scheduling, the fixed-snapshot node, Kubernetes resources, persistent storage, and result-history publication.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.63%. Comparing base (36c289c) to head (9601b24).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3944      +/-   ##
==========================================
- Coverage   74.77%   74.63%   -0.15%     
==========================================
  Files         461      461              
  Lines       40928    40928              
==========================================
- Hits        30605    30546      -59     
- Misses       8223     8282      +59     
  Partials     2100     2100              
Flag Coverage Δ
jsonv2 77.42% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from a70bc65 to dcb6bf9 Compare August 17, 2026 15:05
@danielntmd
danielntmd changed the base branch from danielntmd/benchmark-image to dat/bench-full-flow August 17, 2026 17:46
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch 5 times, most recently from a19dee7 to 17d0c39 Compare August 17, 2026 22:19
@danielntmd danielntmd changed the title feat(bench): benchmark supported RPC endpoints feat(bench): add managed RPC benchmark flow Aug 17, 2026
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 17d0c39 to 72969ea Compare August 18, 2026 04:30
Base automatically changed from dat/bench-full-flow to main August 18, 2026 09:14
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 72969ea to a39478c Compare August 18, 2026 19:26
@danielntmd
danielntmd deployed to Development August 18, 2026 19:32 — with GitHub Actions Active
@danielntmd danielntmd changed the title feat(bench): add managed RPC benchmark flow feat(bench): add managed nightly RPC benchmarks Aug 18, 2026
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from a39478c to 49fe138 Compare August 18, 2026 20:04
@danielntmd
danielntmd deployed to Development August 18, 2026 20:12 — with GitHub Actions Active
@danielntmd
danielntmd marked this pull request as ready for review August 18, 2026 21:36
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 49fe138 to 70d5234 Compare August 18, 2026 21:36
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 8m 14s —— View job


Review: managed nightly RPC benchmarks

  • Read repo guidelines (no root CLAUDE.md in this checkout)
  • Review shell scripts (gen-all.sh, run-all.sh, nightly.sh, test-case-artifacts.sh)
  • Review Dockerfile, compose, workflow
  • Review run.js and static viewer
  • Post inline findings + summary

Solid, careful work — atomic staged generation with mv -T, checksum-verified reuse, case-ID validation against path escape, signal forwarding to the live k6 process, --summary-export validated before a case is called passed, and a genuinely useful test script that covers the failure paths (empty summary, failed warmup, missing corpus, wrong snapshot, mutated corpus, TERM propagation). The Docusaurus keep_files: true deploy makes the docs/static viewer + Argo-published runs/ coexist correctly. No correctness bug that would corrupt a measurement or leak resources.

The findings below are mostly about operability of the reuse scheme — what happens on the days when inputs change.

Blocker

None.

Important

  1. generate doesn't preflight (nightly.sh:49) — manifests stamp SNAPSHOT_ID/SHA256/BLOCK_NUMBER as provenance that was never verified against the node (only rpcVersion is cross-checked). Every reuse decision and published result then trusts it. compose.yaml:20 passes EXPECTED_CHAIN_ID to the corpus service, where it's currently unused — the intent looks like it was already there.
  2. An incompatible case is terminal (gen-all.sh:214-223) — reuse fails and generate_managed refuses to write over an existing dir, so a snapshot rotation (a routine event here) fails all 23 cases until someone manually clears the PVC. Keying storage by identity hash (<case>/<hash>/) or adding a supported prune/regenerate path would fix it.
  3. Corpus identity is the raw argv vector (gen-all.sh:62-67) — --concurrency is included even though it can't affect content (newSeededRand(seed, i) + WithMaxGoroutines), and --count 2 vs --count=2 mismatch, so a README-generated PVC and a nightly-generated one are mutually unusable.
  4. WARMUP_MAX_DURATION is a failure trigger, not a bound (run-all.sh:168-184 + run.js:23-25) — hitting the 10m cap fails the count==N threshold, so the case is failed and measurement is skipped. Likely to bite traceBlockTransactions/traceTransaction/getStorageProof at 200 sequential iterations over 10k sampled mainnet blocks.
  5. summary.json records what, not how (run-all.sh:302-316) — no --vus/--duration and no per-case corpus identity, so dashboard trends aren't comparable across a VUS or corpus --count/--seed change.

Nit

  1. curl in rpc_result has no --connect-timeout/--max-time (nightly.sh:15-23) — a stalled node hangs the nightly until Argo's deadline.
  2. Empty/malformed corpus → % 0NaN index → every VU POSTs undefined (run.js:36); reads as 100% check failures instead of "bad corpus".
  3. Stale per-case artifacts for removed catalog entries survive in a reused RESULTS_DIR and get published (run-all.sh:139-142).
  4. verify_managed's single error message covers both a real mismatch and a missing/corrupt corpus.json (gen-all.sh:112-117), pointing the operator at the wrong thing.
  5. chown k6:k6 /corpus /results is defeated by root-owned PVC mounts (Dockerfile.benchmark:15-18) — worth documenting the required fsGroup.
  6. entry.summary from history.json is unvalidated before fetch (index.html:123-131), and render() assumes run/juno/snapshot exist.
  7. Not commented inline: WARMUP_MAX_DURATION isn't validated the way WARMUP_ITERATIONS/STRICT_CHECKS are (run-all.sh:32-42); .staging/ dirs from a killed generation are never reaped (gen-all.sh:124); run-all.sh reports EXPECTED_CHAIN_ID/SNAPSHOT_BLOCK_NUMBER as juno.chainId/juno.blockNumber without asking the node (safe under nightly.sh, misleading standalone); and 23 cases × (warmup + 30s) can approach hours, so the Argo job needs a deliberate deadline.

test-case-artifacts.sh doubling as driver and as the stub for gen-all/run-all/curl/k6/corpus-gen (dispatching on basename $0 + STUB_*) is effective but dense; a small stubs/ dir would read better as this grows.

Note: I reviewed by reading — the sandbox here blocked running bash -n, the test script, and docker compose config, so I didn't independently execute the validation listed in the PR description.
• branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/nightly.sh
Comment thread bench/rpc/gen-all.sh
Comment thread bench/rpc/gen-all.sh
@danielntmd
danielntmd deployed to Development August 18, 2026 21:44 — with GitHub Actions Active
Comment thread bench/rpc/run-all.sh
Comment thread bench/rpc/run-all.sh
Comment thread bench/rpc/nightly.sh
Comment thread bench/rpc/run.js
Comment thread bench/rpc/run-all.sh
Comment thread bench/rpc/gen-all.sh
Comment thread bench/rpc/Dockerfile.benchmark
Comment thread docs/static/benchmark-results/rpc/index.html
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 70d5234 to 9fc4bb4 Compare August 18, 2026 22:22
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 3m 41s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff against the previously reviewed revision
  • Verify the fixes marked "addressed"
  • Review the new changes for regressions
  • Post inline findings + summary

The two items you marked addressed are genuinely fixed, and both came with test coverage rather than just code:

  • curl timeouts--connect-timeout 5 --max-time "${RPC_TIMEOUT:-30}" (nightly.sh:18), asserted by the stub at test-case-artifacts.sh:44-45 and exercised with a non-default RPC_TIMEOUT=17 at line 177. A stalled node now fails fast instead of burning the Argo deadline.
  • summary.json records how, not just whatload: {vus, duration} at the run level plus per-case corpus: {sha256, count, seed, commit} lifted from the already-validated manifest (run-all.sh:271-283, 324), asserted against the real manifest at test-case-artifacts.sh:264-281. chmod 0644 before the mv is a nice touch for a file Argo has to serve.

Everything else stands as you left it — the deferrals (argument normalization, incompatible-case recovery, the warmup cap, corpus shape assertion, the viewer's entry.summary validation) are all reasonable calls given Argo owns snapshot rotation and PVC lifecycle. Nothing from the previous round regressed, and I found no new correctness bug: the reuse path, checksum verification, signal forwarding, and the missing-summary failure path are unchanged and still covered.

Four small things on the new code:

Blocker

None.

Important

  1. load is a claim, not an observation (run-all.sh:324) — VUS/DURATION come from the environment, while the profile k6 actually ran comes from K6_ARGS; nothing connects them. The new test proves the gap: run_managed exports VUS=50 DURATION=30s and then calls run-all.sh with no k6 flags, yet asserts load == {vus: 50, duration: "30s"}. It agrees under nightly.sh only because that caller happens to build the flags from the same variables. Reading it back from k6's exported summary (metrics.vus_max) would make it an observation.

Nit

  1. Unvalidated VUS costs the whole artifact (run-all.sh:311) — WARMUP_ITERATIONS and STRICT_CHECKS are shape-checked, VUS isn't. VUS=5O → k6 rejects the flag → all cases fail → --argjson vus "5O" aborts jq → set -e → the EXIT trap deletes the temp file and no summary.json is written. A typo turns a self-explaining failed run into a 404 in the viewer.
  2. The viewer doesn't render the new fields (index.html:89) — load and per-case corpus are now in the artifact but never displayed, so the operator staring at a p99 jump still can't see that VUS or the corpus changed. Two lines closes the loop, and both degrade to on older summaries.
  3. The README example still uses space-form flags (README.md:140) — --count 10000 here vs --count=10000 in nightly.sh:53. Identical corpora, non-matching manifests; since normalization is deferred and an incompatible case is terminal by design, a locally primed PVC is a manual rm -rf away from recovery. Also undocumented: RPC_TIMEOUT, VUS, DURATION, and the new summary fields.

Note: as last time, the sandbox blocked bash -n, test-case-artifacts.sh, and docker compose config, so this is a reading review — I didn't independently execute the validation listed in the description.
· branch danielntmd/benchmark-endpoints

@danielntmd
danielntmd requested a review from infrmtcs August 18, 2026 22:23
Comment thread bench/rpc/run-all.sh Outdated
juno: {version: ($juno_version | optional), imageDigest: ($juno_image | optional), chainId: ($chain_id | optional),
blockNumber: $block_number, rpcVersion: ($rpc_version | optional)},
snapshot: {id: ($snapshot_id | optional), sha256: ($snapshot_sha | optional), blockNumber: $block_number},
load: {vus: $vus, duration: ($duration | optional)},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important: load is a self-reported claim from the environment, not an observation of what k6 actually ran, so it can disagree with the measurement it annotates.

VUS/DURATION are read from the environment here, while the load profile actually applied comes from K6_ARGS (line 28 → line 203). Nothing ties the two together — run-all.sh never reads VUS for anything except this field.

The new test demonstrates the divergence: run_managed (test-case-artifacts.sh:205) exports VUS=50 DURATION=30s and then invokes run-all.sh "${config}" http://localhost with no k6 flags at all, yet test-case-artifacts.sh:270 asserts .load == {vus: 50, duration: "30s"}. The artifact records a load profile that was never applied, and the test locks that in as correct.

Under nightly.sh the two agree today because run() builds the flags from the same variables, but that's a coincidence of the caller, and this artifact is published as run provenance for exactly the "why did p99 move?" question. Deriving it from what k6 reported (the exported summary carries a vus_max metric — worth confirming the shape against k6 1.7's --summary-export) or, failing that, from "${K6_ARGS[@]}" would make the field an observation instead of a claim.

Fix this →

Comment thread bench/rpc/run-all.sh Outdated
Comment thread docs/static/benchmark-results/rpc/index.html
Comment thread bench/rpc/README.md Outdated
@danielntmd
danielntmd deployed to Development August 18, 2026 22:28 — with GitHub Actions Active
Package corpus generation and k6 replay in a benchmark image with a stable preflight, generation, and run interface for scheduled execution.

Reconcile per-case corpora on persistent storage using snapshot identity, RPC version, generator metadata, and checksums. Warm and replay active cases sequentially, preserve per-case failures, enforce strict k6 checks, and emit compact summaries with canonical load and per-case corpus provenance. Bound preflight RPC requests so stalled nodes fail predictably.

Publish latest and commit-addressed images through CI, provide Compose for local execution, and include a static viewer for latest and historical results. ArgoCD supplies the schedule, fixed-snapshot node, persistent volumes, runtime image identities, and result publication.
@danielntmd
danielntmd force-pushed the danielntmd/benchmark-endpoints branch from 9fc4bb4 to 9601b24 Compare August 18, 2026 23:28
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 4m 11s —— View job


Re-review: managed nightly RPC benchmarks

  • Diff the new push against the previously reviewed revision
  • Verify the four items raised last round
  • Review the new changes for regressions
  • Post inline findings + summary

Three of the four items from the last round are fixed, each with a test or a doc change behind it:

  • Unvalidated VUS--argjson--arg plus a jq-side test("^[1-9][0-9]*$") guard (run-all.sh:311, 324). A typo now yields load.vus: null instead of aborting jq and losing the whole artifact, and test-case-artifacts.sh:303-310 pins that: VUS=5O STUB_K6_FAIL=1 still writes a summary.json with run.status == "failed".
  • Viewer doesn't render the new fieldsLoad metadata row (index.html:90) and a Corpus column fed by item.corpus?.sha256?.slice(0, 12) (index.html:37, 95). Header and body cell counts line up at 14, and both degrade to on summaries that predate the fields.
  • README space-form flags--count=10000 --seed=1 --concurrency=32 now matches nightly.sh:53-55 exactly, so a locally primed PVC produces a manifest the nightly will accept. RPC_TIMEOUT/VUS/DURATION and their defaults are documented at README.md:189-190, and the nightly boundary now mentions the load and per-case corpus fields.

The fourth (load as claim vs. observation) is half-closed: run_managed now passes --vus=50 --duration=30s (test-case-artifacts.sh:209), so the test no longer asserts a profile that was never applied, and the README calls it "the configured load." The derivation itself is unchanged — see below.

Nothing regressed. The reuse path, checksum verification, mv -T staging, signal forwarding, and the missing-summary failure path are all as they were, and I found no new correctness bug.

Blocker

None.

Important

  1. load still disagrees with the flags in this repo's own CI (run-all.sh:311-325) — the smoke test at rpc-benchmark.yaml:74-77 sets SUMMARY_FILE and runs --vus 1 --iterations 1 with no VUS in the environment, so the artifact records load: {vus: null, duration: null} for a run that used 1 VU. SUMMARY_FILE is a public knob, so any caller that isn't nightly.sh hits this. Reading it back from k6's exported summary (metrics.vus_max), or preferring --vus/--duration parsed out of K6_ARGS, would close it.

Nit

  1. A malformed VUS is discarded silently (run-all.sh:325) — coercing to null is the right call, but load.vus: null is byte-identical to "no VUS configured," so the artifact meant to explain the failure doesn't. A warning: line next to the WARMUP_ITERATIONS/STRICT_CHECKS guards keeps both.
  2. Load renders — VUs · — instead of (index.html:90) — the guard tests the container, not its contents, and an all-null load is exactly what both CI smoke runs produce.
  3. SUMMARY_FILE is undiscoverable (run-all.sh:11-13) — the usage block omits SUMMARY_FILE, STRICT_CHECKS, VUS, and DURATION, and the README's run-all.sh example (README.md:148-161) doesn't set SUMMARY_FILE either, so following it produces no summary.json. It's the variable Argo's whole contract rests on.

Not worth an inline comment: the previously-noted deferrals (argument normalization, incompatible-case recovery, the warmup cap, corpus shape assertion, entry.summary validation) are unchanged and remain reasonable given Argo owns snapshot rotation and PVC lifecycle. JUNO_COMMIT being baked in via ARG/ENV (Dockerfile.benchmark:20-21) rather than passed through compose is correct — the run() guard gives a clear error if an image is built without it.

Note: as in both prior rounds, the sandbox blocked bash -n, test-case-artifacts.sh, docker compose config, and even writing a scratch file, so this is a reading review — I did not independently execute the validation listed in the description.
· branch danielntmd/benchmark-endpoints

Comment thread bench/rpc/run-all.sh
Comment thread bench/rpc/run-all.sh
Comment thread docs/static/benchmark-results/rpc/index.html
Comment thread bench/rpc/run-all.sh
@danielntmd
danielntmd deployed to Development August 18, 2026 23:35 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant