Enhancements to Logstash Benchmarking Tool #10253
Conversation
1. Custom Data Sets 2. Added heap used statistics to results
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Jenkins please test this |
|
@jakelandis @original-brownbear Could you take a look at this PR. It contains changes to the logstash benchmarking tool to support custom configs. Also, made changes to output heap usage stats. If you prefer, I can break these up into separate PR's. That would make it easier to review. |
| @@ -32,7 +32,7 @@ | |||
| public static final String TEST_CASE_PARAM = "testcase"; | |||
|
|
|||
| public static final String TEST_CASE_HELP = | |||
| "Currently available test cases are 'baseline' and 'apache'."; | |||
| "Currently available test cases are 'baseline', 'apache' and 'custom'."; | |||
There was a problem hiding this comment.
Maybe we should just remove this param and simply allow setting the new config param instead to simplify things a little?
There was a problem hiding this comment.
We could, do we not need to maintain backwards compatibility ?
There was a problem hiding this comment.
Nah, I don't think so :)
There was a problem hiding this comment.
Ok, in that case, can get rid of this param.
There was a problem hiding this comment.
Hi @original-brownbear, we need to support 3 types of configs / test cases -
- Baseline - This uses the logstash generator plugin to generate random input data. No config file is needed for this case.
- Apache - uses the sample apache config and a test data set downloaded from the provided url. Config file alone is not sufficient in this case, the data set is needed as well.
- Custom - Need path to the custom config to run benchmarking against.
So, for #1, we dont have the path to a config file to specify. For #2, we need to invoke a custom apache class that takes care of downloading the apache data set.
I can combine all three of the test cases above into a single input flag - "config". Config value would be "baseline", "apache" or path to a custom file.
cc @jakelandis @jsvd
There was a problem hiding this comment.
Went through the code again. Given the different handling required for the different test cases - baseline, apache and custom, I dont think it is a good idea to have a single config flag for all three, since the data type of the input will be different in each case.
See @jakelandis comments in this issue - "Add in an option for --testcase=custom and expose a --conifg-dir option such that, assuming I can figure out how to feed data in, I can use this for any test case."
| @@ -0,0 +1,28 @@ | |||
| input { | |||
There was a problem hiding this comment.
I don't think we should ship this as a default test case since it requires a running Kafka instance with specific data in it for input and output.
There was a problem hiding this comment.
Makes sense, will remove.
There was a problem hiding this comment.
Have removed this file.
|
@original-brownbear thanks for the review. Will make the changes, is there anyone on the logstash team that can review and approve the PR ? |
|
pinging @jsvd for that :) Not sure who's handling this code now. |
|
@jsvd @original-brownbear @jakelandis Wondering if any of you could approve this PR ? @original-brownbear has already taken a first look at the changes, so this should not take long. I'm excited for this to be my first logstash contribution, your time would be much appreciated ! Thanks in advance. |
Consistent code formatting
|
@danhermann, thanks very much for reviewing. Have approved your pull request. So, we can go ahead and merge this PR. |
There was a problem hiding this comment.
@aagupta1, I've gone ahead and merged it. Thanks for the contribution!
* Adding support for - 1. Custom Data Sets 2. Added heap used statistics to results
* Adding support for - 1. Custom Data Sets 2. Added heap used statistics to results
* Adding support for - 1. Custom Data Sets 2. Added heap used statistics to results
Adding support for -