-
Notifications
You must be signed in to change notification settings - Fork 313
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
Add an asyncio-based load generator #916
Conversation
With this commit we add an async load generator implementation. This implementation is work in progress, extremely incomplete and hacky. We also implement an async compatibility layer into the previous load generator which allows us to compare both load generator implementations in realistic scenarios.
With this commit we bump the minimum required Python version to Python 3.6 (thus dropping support for Python 3.5). Python 3.5 will be end of life on September 13, 2020 (source: [1]). We also intend to use several features that require at least Python 3.6 in future versions of Rally thus we drop support for Python 3.5 now. [1] https://devguide.python.org/#status-of-python-branches
With this commit we change Rally's internal implementation to always use the async code path so runner implementations stay the same.
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Due to elastic/rally#852 we will implement a compatibility layer in the current load generator that will also use the asyncio API and thus requires custom runners to be registered differently (by specifying `async_runner=True`). Rally's runner registry will also expose a new attribute `async_runner` that is set to `True` if Rally requires runners to be registered as described above. With this commit we introduce a (temporary) compatibility layer for all custom runners that allows older Rally versions to work with the classic runners and newer Rally versions with the async runners. Relates elastic/rally#852 Relates elastic/rally#916
Notes to reviewersGeneralThis PR already includes #905 to drop Python 3.5 support. As soon as the other PR is merged to master I'll resolve conflicts but please ignore any diffs that are due to that PR. Planned follow-up workI plan to work on the following items after this PR has been merged:
UsageThe new subcommand only supports benchmarking and requires to use the Here are some example invocations:
Note that also the current load generator has changed (internally). |
The PR build (Python 3.7) just failed with:
which is rather odd given that we actually have a guard that checks that before invoking the function: rally/esrally/mechanic/launcher.py Lines 230 to 231 in 8a2ebe8
Stopping a node is also single-threaded so I'm a bit at a loss why the @elasticmachine test this please |
This uncovered indeed a real issue that only happens when we run against a Docker container against an in-memory metrics store. I have pushed #918 to address this. |
Additional note to reviewers: on an existing Rally installation, before reviewing this PR it's advisable to delete your .venv and recreate with |
While testing against an Elasticsearch launched with an increased write thread pool
I invoked Rally using
and hit an error at around
Looking at the logs there was a request timeout, so probably Rally did what it should given that Rally stack trace
|
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.
Massive work thank you!
I took an initial pass -- not super deep admittedly -- of the code, except for the tests, and so far everything seems sane to me.
I left some comments about observations re: the failure situations.
I'll take another pass at a later stage.
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.
I tortured tested in a Vagrant env for backwards compatibility
and things went fine.
From my perspective we are good to merge.
Thanks for your review and also finding the reporting issue with the network timeout. I dug and it turns out that when a |
This reverts commit c8f2de7.
With this commit we add a new experimental subcommand `race-aync` to Rally. It allows to specify significantly more clients than the current `race` subcommand. The reason for this is that under the hood, `race-async` uses `asyncio` and runs all clients in a single event loop. Contrary to that, `race` uses an actor system under the hood and maps each client to one process. As the new subcommand is very experimental and not yet meant to be used broadly, there is no accompanying user documentation in this PR. Instead, we plan to build on top of this PR and expand the load generator to take advantage of multiple cores before we consider this usable in production (it will likely keep its experimental status though). In this PR we also implement a compatibility layer into the current load generator so both work internally now with `asyncio`. Consequently, we have already adapted all Rally tracks with a backwards-compatibility layer (see elastic/rally-tracks#97 and elastic/rally-eventdata-track#80). Closes #852 Relates #916
With this commit we add a new experimental subcommand
race-aync
to Rally. Itallows to specify significantly more clients than the current
race
subcommand.The reason for this is that under the hood,
race-async
usesasyncio
and runsall clients in a single event loop. Contrary to that,
race
uses an actorsystem under the hood and maps each client to one process.
As the new subcommand is very experimental and not yet meant to be used broadly,
there is no accompanying user documentation in this PR. Instead, we plan to
build on top of this PR and expand the load generator to take advantage of
multiple cores before we consider this usable in production (it will likely keep
its experimental status though).
In this PR we also implement a compatibility layer into the current load
generator so both work internally now with
asyncio
. Consequently, we havealready adapted all Rally tracks with a backwards-compatibility layer (see
elastic/rally-tracks#97 and rally-eventdata-track#80).
Closes #852