Test: Start nodes using node.bench true to enable benchmark api #5920
Conversation
|
I jsut checked and saw there are no tests for this API at all in out code base. I don't like that we set this by default, I think the REST tests are an afterthought and this shoudl be tested internally first if it's such a special case. |
|
I agree making every node a bench node is not a good idea, the idea is then to have a single client node that can be used for benchmarks as well. In order for this to be possible though we need to be able to have client nodes within our test cluster, which is why I created #5949. Once we get that in we can make the client node a bench node too. |
|
I just updated this PR that randomly adds the node.bench setting to the eventually present client node. The execution of the benchmark tests now depends on the feature |
| @@ -126,6 +127,8 @@ | |||
| static final int DEFAULT_MIN_NUM_CLIENT_NODES = 0; | |||
| static final int DEFAULT_MAX_NUM_CLIENT_NODES = 1; | |||
|
|
|||
| static final boolean DEFAULT_DISABLE_BENCH_NODES = false; | |||
There was a problem hiding this comment.
DISABLES_XXX=false is a double negation and can easily get me confused. Should it be ENABLE_XXX instead?
There was a problem hiding this comment.
I was waiting for this comment :) the reason why i went for the double negation is that saying ENABLE you give the illusion that you can control whether the feature is going to be enabled or not. while here we randomize it if there's at least a client node and the feature is not disabled. For the very same reason I used a boolean instead of a number.
There was a problem hiding this comment.
know what? I could do ENABLE_RANDOM_BENCH_NODES. Like it?
|
I don't have enough experience with the rest tests to comment on whether this is the best way to do it but it looks good and I think it's important to enable tests on the benchmark API ASAP so I'd vote to get this in. We can iterate on this change later on if necessary. |
|
Looks good. |
…and re-enabled REST benchmark tests based on number of bench nodes available In our REST tests we already have support for features and skip sections that allow to skip tests if a feature is not supported. We can then add a skip section based on the benchmark feature to the benchmark tests and execute them only when they are supported, knowing that they need at least a node with node.bench settings within the cluster. We can check that this requirement is met by calling the nodes info api. This way we can dynamically decide whether to execute those tests or not and we don't need to have a node.bench around all the time. In fact, given that the REST tests use the GLOBAL cluster, we want to be able to randomize settings as much as possible and run tests against default settings as well. Also, this mechanism can be easily supported by the external cluster implementation that is used during the release process. Introduced ability to disable benchmark nodes which is needed by BenchmarkNegativeTest.
clintongormley
changed the title
Enabling benchmark apis is required to run our REST tests for it. Added the parameter to node creation in
TestClusteras well in the release script, when starting the node to run the smoke tests against.Closes #5910