-
Notifications
You must be signed in to change notification settings - Fork 116
Implement incremental rate type #428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement incremental rate type #428
Conversation
|
@albertoperdomo2 Should #291 be closed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we don't like to add top-level arguments that only map to a specific use-case. Many of the other components have a "kwargs" argument so it might make sense to add a --profile-kwargs. Also --rate needs to map to something so maybe make it increment_factor (or start_rate; whichever it makes more sense to sweep over).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option might be to modify --rate to take tuples of (start_rate, increment_factor).
cc: @markurtz @jaredoconnell to weigh in on this.
| else: | ||
| increment = 1.0 / next_rate | ||
|
|
||
| self._process_offset += increment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strategies are shared across threads/processes, any assignments need to be atomic or locked with self._processes_lock. See other Strategies for examples.
6cb8c60 to
ad95da4
Compare
|
Do you think your requirements regarding this could be handled with the |
|
@jaredoconnell I think the best bet would be to implement it via rampup_duration inside of constant, but maybe @sjmonson has something else to add. IMO this PR can be closed given #549 as I have left it unattended for way to long. Thanks Jared and Sam! |
|
Superseeded by #549 |
## Summary Simply allows a linear rampup of the constant rate profile. ## Test Plan The simplest test is to run a short constant test with 4 requests per second, with a long rampup. You can see how it ramps as expected. There are also new tests. ## Related Issues Fulfills part of the goals of #428 --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes AI-assisted code completion - [x] Includes code generated by an AI application - [x] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
Summary
This PR adds a new rate type
incrementalto the suite of benchmarks. This new rate type starts at a specified initial rate--start-rateand linearly increases the request rate over time by--increment-factor, offering an optional rate limit--rate-limitto cap the maximum rate. This simulates, as the name implies, load ramp up along time.Reimplementation of #291 after the new release.
Details
--start-rate,--increment-factorand--rate-limit.IncrementalProfileclass.AsyncIncrementalStrategyscheduler strategy that handles the logic and includes the optional initial burst.Test Plan
Related Issues
Use of AI
## WRITTEN BY AI ##)