Skip to content

Conversation

@ashley
Copy link
Contributor

@ashley ashley commented Jan 17, 2020

Changing the description of -c to Number of workers to run concurrently... and -q to in queries per worker per second..... This will clarify that the c flag is used to define the number of threads and -q is actually the number of requests per worker per second.

Motivated by #140 #143 and #158

Use Cases:

  • Someone who uses the duration (-z) flag, and want to set the total number of queries per second:
    • Recognize that QPS (queries per second) is -c times -q
  • Someone who wants to send 2,000 total number of queries without changing -n:
    • Recognize that -c is not the total number of requests and would set -c as 10 because the default -n is 200 so 200 * 10 = 2,000 queries

This makes more sense as a first impression, but I'm open to any recommendations!

hey.go Outdated
-c Number of workers to run concurrently. Total number of requests cannot
be smaller than the concurrency level. Default is 50.
-q Rate limit, in queries per second (QPS). Default is no rate limit.
-q Rate limit, in queries per worker per second. Default is no rate limit.
Copy link
Owner

Choose a reason for hiding this comment

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

This is actually per request per worker per second. Is it possible to say "QPS per worker"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

@ashley ashley requested a review from rakyll January 18, 2020 03:19
@rakyll
Copy link
Owner

rakyll commented Jan 18, 2020

Thanks so much!!!

@rakyll rakyll merged commit 36f181a into rakyll:master Jan 18, 2020
@ashley ashley deleted the ashley/clarify-parameter-c-description branch January 18, 2020 22:45
@ashley
Copy link
Contributor Author

ashley commented Jan 18, 2020

I realized that the README.md should also be updated 😃

@rakyll
Copy link
Owner

rakyll commented Jan 21, 2020

Would you be interested in sending a follow up PR to update the README?

@ashley
Copy link
Contributor Author

ashley commented Jan 23, 2020

@rakyll Opened #191 for review

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