Add workload descriptions to YCSB benchmark series names - #1391
Conversation
The github-action-benchmark dashboard keys each chart series by name, but
the YCSB converter emitted bare names ("load", "workload A", "C read p99"),
so the trend titles/legends were not self-explanatory. Source the workload
description from the results JSON (falling back to the authoritative
WorkloadSpec in workload.mts) and append it to each throughput and latency
series name, e.g. "workload A — Update heavy (50% read / 50% update)" and
"A update p99 — update heavy".
Renaming series starts new lines on the dashboard (series are keyed by
name); the old-named series stop updating. This is expected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the YCSB benchmark JSON conversion script by incorporating workload descriptions and short tags into the output benchmark names. The reviewer provided valuable feedback on improving robustness, specifically recommending defensive checks to prevent runtime errors if the load phase or workloads array are missing, and suggesting a way to avoid redundant suffix tags when the workload description falls back to the workload name.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed the follow-up commits (--reps median-of-N, keyspace carry-forward, reps clamping); no blockers found. The |
Each workload can run N repetitions; the reported point is the median rep by throughput, dampening healthy-run jitter so the nightly trend is robust to a single degenerate rep. Warmup stays once per workload (the dataset is hot after the first rep). reps defaults to 1, so single-node behavior is unchanged; the cluster runner opts into 3. Result JSON gains additive reps/repThroughputs fields, which the convert + validate scripts ignore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed a follow-up on this branch: a The cluster runner (harper-pro #425) opts into — Claude Opus 4.8 |
Cross-model review flagged that insert-bearing workloads (E, D) re-inserted the same keys each rep — a fresh KeyState per rep seeded at config.records made reps 2+ PUT-overwrite the keys rep 1 inserted instead of growing the keyspace. Carry the acknowledged readable count forward between reps (new KeyState.keyCount getter) so each rep allocates fresh insert keys, mirroring one continuous run. Read/scan-only workloads never advance it, so they're unaffected. The carry stays within the existing key-width budget (worst case records + reps × insertFraction × ops < records + ops). Adds unit coverage: medianByThroughput (middle rep, ignores a degenerate run, conservative lower-middle on even counts, single-rep no-op) and the KeyState frontier the carry-forward relies on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two hardening fixes from cross-model review: - parseOptions now floors and clamps --reps to >=1, so a non-numeric/zero/negative value can't leave it NaN — that would make the rep loop never run and medianByThroughput crash on an empty set. Covered by a parseOptions test. - keyWidth budgets for records + reps × opsPerWorkload (every rep all-inserts, the pessimistic bound) so the carried-forward keyspace can never overflow the padded key width, regardless of a workload's insert fraction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… tags - to-benchmark-json: omit the `load` series instead of throwing when `results.load` is absent (e.g. a run against a pre-loaded dataset). - describeWorkload now returns undefined when no real description exists, so an unknown workload no longer renders "workload FOO — FOO" / a "foo" latency tag — the suffix/tag are dropped instead. - Extract pure `convert()` and gate `main()` behind an import.meta guard so the logic is unit-testable; add to-benchmark-json.test.mts covering the normal run, missing-load, no-description, and result-description cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l runs, timestamp x-axis - Merge pre-#1391 bare series names into their descriptive equivalents so each metric is one continuous trend line (was double-plotting old + new after #1391). - Drop partial-run outliers (commit points with fewer benches than a full run). - Custom index.html: timestamp x-axis, commit SHA + click-to-open in tooltip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
The nightly YCSB cluster trend dashboard (github-action-benchmark) emitted bare chart-series names —
load,workload A,C read p99— so the dashboard titles and legends weren't self-explanatory. This appends a brief workload description to each throughput and latency series name in the converter.The description is sourced from the results JSON's per-workload
description(written byharness.mts), falling back to the authoritativeWorkloadSpec.descriptioninworkload.mtsso labels stay accurate even for results that predate thedescriptionfield.Resulting names (confirmed against
workload.mts):load — bulk insertworkload A — Update heavy (50% read / 50% update)workload B — Read mostly (95% read / 5% update)workload C — Read only (100% read)workload E — Short ranges (95% scan / 5% insert)workload F — Read-modify-write (50% read / 50% read-modify-write)A update p99 — update heavy(short tag = the phrase before the op-mix parenthesis)Attention
to-benchmark-json.mtsis a standalone CLI invoked by the workflow (not part of the unit-test tree); verified by running it against a sample results file. TheshortTag/describeWorkloadhelpers are exercised through that run.Coordination
Paired with HarperFast/harper-pro#425, which guards the publish step against partial/cancelled runs and bumps the
coresubmodule pointer to this commit.Generated by Claude (Opus 4.8).
Follow-up addition (same branch) —
--repsmedian-of-N in the harnessLayered on top of the series-name change above, to reduce the cluster bench's healthy-run jitter.
harness.mtsgains a--repsflag (default 1, so single-node behavior is unchanged). Whenreps > 1, each workload runs N times and the reported point is the median rep by throughput (medianByThroughput). Returning a whole rep — rather than computing each metric's median independently — keeps the reported throughput and its latency block internally consistent (they come from one real run). A single degenerate rep sorts to an end of the set and is never selected; for an even count we take the lower-middle (the more conservative throughput).--reps=3.reps/repThroughputsfields.to-benchmark-json.mtsandvalidate-benchmark-json.mtsonly readname/description/throughput/latency, so both ignore the new fields and stay compatible (verified by round-tripping a synthetic 5-workload result through convert + validate).Attention
medianByThroughputis not covered byworkload.test.mts(it lives in the standalone harness CLI, like the existing convert helpers); verified via a direct round-trip script. Existingworkload.test.mtsstill passes (10/10).Generated by Claude (Opus 4.8).