Indices stats options #6390
Indices stats options #6390
Conversation
The "groups" and "types" parameters to indices stats were being set too early, so could end up being cleared.
The "fields" parameter to indices stats accepts wildcards, but the "groups" and "types" parameters don't.
* metrics * level * fields * groups * types
|
I've added YAML tests rather than Java test - are Java tests still required, even if they just duplicate the YAML tests? |
| - do: | ||
| indices.stats: {} | ||
|
|
||
| - match: { _shards.total: 20 } |
There was a problem hiding this comment.
I'm not sure we assume we have 20 shards in the index?
There was a problem hiding this comment.
Yes we can. I'm checking the total number of defined shards, not active shards. The test creates two indices with default settings == 20 shards.
There was a problem hiding this comment.
I think we randomize these in java land. If we don't do it now, the day is not far away..
There was a problem hiding this comment.
We do randomize it....just run the tests and you'll see failures mvn clean test -Dtests.class=*.ElasticsearchRestTests or just run the ElasticsearchRestTests test form your IDE ;)
There was a problem hiding this comment.
Good to know - I've pushed a change to make the tests more determinative in this case :)
|
Looking good! Left some comments |
… an array for completion fields, fielddata fields, types and groups
|
@bleskes I've updated the PR in response to your comments. |
|
@clintongormley changes look good. What about the number of shards tests? |
|
@bleskes pushed changes to make the number of shards predetermined. |
|
LGTM! |
|
My two cents on testing: the REST bug (groups and types were being ignored) can only be tested from a REST test. Beyond that, I'd love to see some Java test for these changes, although I do see that we have a very good coverage in terms of yaml tests. The reason is simply that we should not rely too much on the REST tests which are a bit of an afterthought and don't necessarily use all the randomizations that we use in our integration tests. Ideally we should be able to test the core without client tests as much as possible, and have a good coverage through Java tests. |
|
+1 to what luca said! I think we should push towards java tests. Can we add those before pushing? |
|
Found a bug in the YAML tests where, thanks to randomization, primaries may not reflect, eg positive query count. Fixed that and added Java tests |
| client().prepareIndex("test1", "type2", Integer.toString(1)).setSource("field", "value").execute().actionGet(); | ||
| client().prepareIndex("test2", "type", Integer.toString(1)).setSource("field", "value").execute().actionGet(); | ||
|
|
||
| client().admin().indices().prepareRefresh().execute().actionGet(); |
There was a problem hiding this comment.
you can use the common method refresh() here which checks also if there are failures in the response.
|
@javanna thanks for looking at the tests. I've pushed a new commit which implements your changes. |
|
Great work @clintongormley !!!! |
Bugs: * "groups" and "types" were being ignored * "completion_fields" as wildcards were not being resolved to fieldnames Enhancements: * Made "groups" and "types" support wildcards * Added missing tests Closes #6390
clintongormley
added
:Data Management/Stats
This PR fixes two bugs in indices stats:
groupsandtypeswere being ignoredcompletion_fieldswith wildcards were not resolving the real field namesIt also allows
groupsandtypesto accept wildcards, likefields,completion_fieldsandfielddata_fieldsAdded YAML tests for indices stats for: indices, metrics, level, fields, types, groups