Skip to content

Number benchmark jobs sequentially in river bench - #1379

Merged
bgentry merged 1 commit into
masterfrom
bg/bench-job-numbers
Sep 29, 2026
Merged

bgentry merged 1 commit into
masterfrom
bg/bench-job-numbers

Conversation

@bgentry

@bgentry bgentry commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

river bench is meant to give each inserted job a unique num arg, but its insert loops update a copy of each args value, so every job is inserted with num: 0.

Both insert paths now write numbered args directly into each reused insert parameter and carry the count between batches. The fixed-count path trims its final batch before numbering it, so the counter advances only for jobs it prepares.

The database test checks persisted args across a real batch boundary, including a shorter final batch. A short comment explains how this catches the original copy-by-value bug.

@bgentry
bgentry force-pushed the bg/bench-job-numbers branch 2 times, most recently from 52b3880 to 50a3e51 Compare September 28, 2026 16:40
@bgentry
bgentry marked this pull request as ready for review September 28, 2026 16:45
`river bench` assigns every inserted job `num: 0` because its insert
loops update copies of the args values. This hides each job's intended
sequence number in both fixed-count and continuous modes.

Write a numbered `BenchmarkArgs` directly into each reused insert
parameter in both loops. Carry the counter between batches, and trim
the final fixed-count batch before numbering so only prepared jobs
advance it.

Check persisted args across a real batch boundary, including a short
final batch. The assertions report the first incorrect number without
dumping thousands of values.
@bgentry
bgentry force-pushed the bg/bench-job-numbers branch from 50a3e51 to 9531245 Compare September 28, 2026 16:49
@bgentry
bgentry requested a review from brandur September 29, 2026 00:03
@bgentry
bgentry merged commit 9573203 into master Sep 29, 2026
15 checks passed
@bgentry
bgentry deleted the bg/bench-job-numbers branch September 29, 2026 11:47
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.

2 participants