Skip to content
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

[TEST] Randomized number of shards used for indices created during tests #5356

Merged
merged 1 commit into from Mar 10, 2014

Conversation

javanna
Copy link
Member

@javanna javanna commented Mar 6, 2014

Introduced two levels of randomization for the number of shards (between 1 and 10) when running tests:

  1. through the existing random index template, which now sets a random number of shards that is shared across all the indices created in the same test method unless overwritten

  2. through createIndex and prepareCreate methods, similar to what happens using the indexSettings method, which changes for every createIndex or prepareCreate unless overwritten (overwrites index template for what concerns the number of shards)

Added the following facilities to deal with the random number of shards:

  • getNumShards to retrieve the number of shards of a given existing index, useful when doing comparisons based on the number of shards and we can avoid specifying a static number. The method returns an object containing the number of primaries, number of replicas and the expected total number of shards for the existing index
  • added assertFailures that checks that a shard failure happened during a search request, either partial failure or total (all shards failed). Checks also the error code and the error message related to the failure. This is needed as without knowing the number of shards upfront, when simulating errors we can run into either partial (search returns partial results and failures) or total failures (search returns an error)
  • added common methods similar to indexSettings, to be used in combination with createIndex and prepareCreate method and explicitly control the second level of randomization: numberOfShards, minimumNumberOfShards and maximumNumberOfShards. Added also numberOfReplicas despite the number of replicas is not randomized (default not specified but can be overwritten by tests)

Tests that specified the number of shards have been reviewed:

  • removed number_of_shards in node settings, ignored anyway as it would be overwritten by both mechanisms above
  • remove specific number of shards when not needed
  • removed manual shards randomization where present, replaced with ordinary one that's now available
  • adapted tests that didn't need a specific number of shards to the new random behaviour
  • fixed a couple of test bugs (e.g. 3 levels parent child test could only work on a single shard as the routing key used for grand-children wasn't correct)
  • also done some cleanup, shared code through shard size facets and aggs tests and used common methods like assertAcked, ensureGreen, refresh, flush and refreshAndFlush where possible

@javanna javanna self-assigned this Mar 6, 2014
assertThat(countResponse.getTotalShards(), equalTo(5));
assertThat(countResponse.getSuccessfulShards(), equalTo(5));
assertThat(countResponse.getTotalShards(), equalTo(numShards.numPrimaries));
assertThat(countResponse.getSuccessfulShards(), equalTo(numShards.numPrimaries));
assertThat(countResponse.getFailedShards(), equalTo(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe have an assertTotalShards() and assertSuccessfulShards() or allows assertAllSuccessful() to provide an amount of shards as well

@jpountz
Copy link
Contributor

jpountz commented Mar 6, 2014

This change looks awesome. I think we should try to get it in very soon so that we don't get merge conflicts or need to track new tests that would be added in the mean time.

@javanna javanna added the test label Mar 6, 2014
@@ -1031,4 +1063,23 @@ public static String randomBytesFieldDataFormat() {
return randomFrom(Arrays.asList("paged_bytes", "fst", "doc_values"));
}

protected NumShards getNumShards(String index) {
MetaData metaData = client().admin().cluster().prepareState().get().getState().metaData();
assert metaData.hasIndex(index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use assertThat here please it will work even if asserts are disabled

@s1monw
Copy link
Contributor

s1monw commented Mar 10, 2014

@javanna I left one small comment! this looks awesome - please go ahead and push it asap!

@javanna javanna added the v2.0.0 label Mar 10, 2014
Introduced two levels of randomization for the number of shards (between 1 and 10) when running tests:

1) through the existing random index template, which now sets a random number of shards that is shared across all the indices created in the same test method unless overwritten

2) through `createIndex` and `prepareCreate` methods, similar to what happens using the `indexSettings` method, which changes for every `createIndex` or `prepareCreate` unless overwritten (overwrites index template for what concerns the number of shards)

Added the following facilities to deal with the random number of shards:
- `getNumShards` to retrieve the number of shards of a given existing index, useful when doing comparisons based on the number of shards and we can avoid specifying a static number. The method returns an object containing the number of primaries, number of replicas and the total number of shards for the existing index

- added `assertFailures` that checks that a shard failure happened during a search request, either partial failure or total (all shards failed). Checks also the error code and the error message related to the failure. This is needed as without knowing the number of shards upfront, when simulating errors we can run into either partial (search returns partial results and failures) or total failures (search returns an error)

- added common methods similar to `indexSettings`, to be used in combination with `createIndex` and `prepareCreate` method and explicitly control the second level of randomization: `numberOfShards`, `minimumNumberOfShards` and `maximumNumberOfShards`. Added also `numberOfReplicas` despite the number of replicas is not randomized (default not specified but can be overwritten by tests)

Tests that specified the number of shards have been reviewed and the results follow:
- removed number_of_shards in node settings, ignored anyway as it would be overwritten by both mechanisms above
- remove specific number of shards when not needed
- removed manual shards randomization where present, replaced with ordinary one that's now available
- adapted tests that didn't need a specific number of shards to the new random behaviour
- fixed a couple of test bugs (e.g. 3 levels parent child test could only work on a single shard as the routing key used for grand-children wasn't correct)
- also done some cleanup, shared code through shard size facets and aggs tests and used common methods like `assertAcked`, `ensureGreen`, `refresh`, `flush` and `refreshAndFlush` where possible
- made sure that `indexSettings()` is always used as a basis when using `prepareCreate` to inject specific settings
- converted indexRandom(false, ...) + refresh to indexRandom(true, ...)
@javanna javanna merged commit d5aaa90 into elastic:master Mar 10, 2014
@javanna javanna added the v1.1.0 label Mar 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests v1.1.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants