-
Notifications
You must be signed in to change notification settings - Fork 124
Rally benchmark #1522
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
Rally benchmark #1522
Conversation
closes #1475 |
|
||
These benchmarks allow you to benchmark an integration corpus with rally. | ||
|
||
For details on how to configure rally benchmarks for a package, review the [HOWTO guide](./docs/howto/rally_benchmarking.md). |
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.
Is this link correct? Trying to find it in this PR.
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.
the link is correct. but I haven't yet written the docs :)
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.
Can we add these to the PR? I was looking for these as I hit some issues testing the PR. Will comment on more on the issue I hit soon.
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.
yes, it's planned to add to this PR: in the description I listed missing docs
:)
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.
Will give it a try when there are docs 🙂
I'm running these commands to do some testing. Few findings:
|
I was not aware of this: during my test I used
We have a separated command for generating rally track without running the benchmark:
👍 |
As a first step, lets make sure it all uses the assets from the package. I agree, unifying the commands might make sense but we could also solve it with docs pointing to it. Key is that the outcome is the same, meaning if I run the rally track "live" or generate the corpus, same data is in. |
In your case I think this comes builtin with rally itself, the intention of this is to defer metric collection until the warm up period ends, so that time is not going to be taken into account for reporting. |
it was in end faster to add saving the rally track and a dry run in this PR than refactor the previous command: I will remove it in a next PR
it will not be exactly the same data: meaning that if I generate the corpus multiple time (running rally) the data will have the same "shape" (cardinality, range, etc), but different randomized value because seeds and time values are based on "now". the generator tool already has the option to pass a seed and "now" from command line, let me know if you want to add to the command in elastic-package as well |
ok, it's a warm up period for metrics collection, that makes sense as it is now then. |
@ruflin I now refresh the index before collecting stats |
If @jsoriano agrees, lets get this PR in rather soonish and then add the corpus templates to some of the integration packages. I expect that this will also provide us some more feedback for iterating on the command itself. Also get it in the hands of the rest of the team to start playing with it. Can we treat the command "beta" for now so we can still make breaking changes to it? |
Co-authored-by: Nicolas Ruflin <[email protected]>
we must merge elastic/package-spec#653 in order to pass CI |
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.
Ok to merge this and iterate, once the package-spec change is released.
before merging this I have to revert the last two commits, but without package-spec released it won't pass CI. |
I just merged the package-spec PR. My assumption is that we would stay on a commit reference but would move it over to the one in package-spec which is merged now instead of a release. And then as soon as a new package-spec is out, we update. |
Updating package spec in a separate PR #1539 |
💚 Build Succeeded
History
cc @aspacca |
adding command to run rally benchmark
system
andrally
: it is no 100% the same, not sure if trying to factorize it and move to common$PATH
@marc-gr: I have a doubt about warm-up period. it is run in a goroutine in the setup method, rally will be executed before that. I'm not sure how does it works for system benchmark. what is it supposed to achieve the warm up period?