add duration flag#1699
Conversation
5545521 to
4a9ebdf
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a9ebdf4c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Accidentally requested copilot review |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a --duration flag to nats bench producer-oriented commands so benchmarks can run for an approximate wall-clock duration by deriving --msgs from --throughput.
Changes:
- Added
--durationflag alongside--throughputand implemented message-count derivation (msgs = throughput × duration) with validation. - Updated the bench cheatsheet with an example using
--duration. - Added unit tests covering
--durationderivation and validation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cli/cheats/bench.md | Documents --duration usage with a concrete example. |
| cli/bench_command.go | Adds the duration flag, derives --msgs from --throughput × --duration, and includes duration in the banner output. |
| cli/bench_command_test.go | Adds tests for processActionArgs() duration/throughput validation and derived message counts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd31a8b0da
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jnmoyne
left a comment
jnmoyne
left a comment
There was a problem hiding this comment.
LGTM, thanks for the contribution.
However in order to merge this the commit needs to be signed, and signed-off.
What you could do here is squash those two commits into one, but do that after having setup commit signing first in your environment, and then in the commit's comment add the Signed-off-by: line at the end of the comment.
e.g. Signed-off-by: Jean-Noël Moyne <jnmoyne@gmail.com> (but with your name and email address).
|
The signature it's requiring is a GPG/SSH signature on the commit |
11803bf to
014026b
Compare
0b43b2c to
e3b164d
Compare
|
Hi @jnmoyne / @ripienaar, is there anything needed from my side? Thanks! My commit was GPG-signed, so I'm not sure why GitHub is showing it as partially verified now. |
|
@tilakraj94 Yes sorry I apologize I made a mistake using GH web page to do a rebase from main while it should have been you doing it and then tried to fix it but it's signed just by me. To clean this up completely can you please rewind your branch to your last commit (signed) and then rebase from main and force push? |
22b9c1b to
1630c51
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1630c51cd7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| // Derive the message count as round(throughput * duration) | ||
| msgs := int64(math.Round(float64(c.throughput) * c.duration.Seconds())) |
There was a problem hiding this comment.
When the derived total is not evenly divisible by --clients, msgsPerClient gives some clients one extra message while each publisher still throttles at throughput/numClients, so duration-based runs can substantially outlive the requested duration. For example, --clients 10 --throughput 1 --duration 11s derives 11 messages; one client gets 2 messages at 0.1 msg/s and therefore runs for about 20s, not 11s. Duration mode needs per-client pacing based on the assigned count and requested duration, or a distribution that preserves the aggregate duration.
Useful? React with 👍 / 👎.
Signed-off-by: Tilak Raj <tilak.raj94@gmail.com>
|
@jnmoyne no worries, i have just rebased with main as well. Please check |
This PR adds a --duration flag to the nats bench producer commands, letting a benchmark run for an approximate wall-clock time instead of a fixed message count. It works as a sub-property of --throughput: when set, the number of messages is derived from the target rate (msgs = throughput × duration) and takes precedence over --msgs; it requires --throughput and errors if the derived count rounds to zero.