Fix scripted metric in ccs #54776
Merged
Fix scripted metric in ccs #54776
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`scripted_metric` did not work with cross cluster search because it assumed that you'd never perform a partial reduction, serialize the results, and then perform a final reduction. That serialized-after-partial-reduction step was broken. This is also required to support elastic#54758.
Collaborator
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
jimczi
approved these changes
Apr 6, 2020
Contributor
jimczi
left a comment
jimczi
left a comment
There was a problem hiding this comment.
Good catch ! I left some minor comments but the change makes sense to me.
| # scripted_metric | ||
| - do: | ||
| search: | ||
| rest_total_hits_as_int: true |
Contributor
There was a problem hiding this comment.
let's use the new format for total hits ?
Member
Author
There was a problem hiding this comment.
Sure! I figured I'd be backporting it and that I'd just uses what is with the rest of the file. But you are right, I really should use the new one.
| MockBigArrays bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); | ||
| if (randomBoolean() && toReduce.size() > 1) { | ||
| // sometimes do an incremental reduce | ||
| // sometimes do an partial reduce |
| /* | ||
| * sometimes simulate cross cluster search by serializing and | ||
| * deserializing the partially reduced result. | ||
| */ |
Contributor
There was a problem hiding this comment.
Should it simply say that we test the serialization of partially reduced result since you have another change that will make this possible in normal search ?
Member
Author
|
Thanks @jimczi ! |
Member
Author
|
run elasticsearch-ci/2 |
Member
Author
|
Thanks @jimczi! |
nik9000
added a commit
to nik9000/elasticsearch
that referenced
this pull request
Apr 7, 2020
`scripted_metric` did not work with cross cluster search because it assumed that you'd never perform a partial reduction, serialize the results, and then perform a final reduction. That serialized-after-partial-reduction step was broken. This is also required to support elastic#54758.
nik9000
added a commit
that referenced
this pull request
Apr 7, 2020
`scripted_metric` did not work with cross cluster search because it assumed that you'd never perform a partial reduction, serialize the results, and then perform a final reduction. That serialized-after-partial-reduction step was broken. This is also required to support #54758.
nik9000
added a commit
that referenced
this pull request
Apr 7, 2020
`testReduceRandom` was bumping up against the serialization that I added in #54776. This makes it use random values that reduce in ways that don't cause the randomized serialization to fail.
nik9000
added a commit
that referenced
this pull request
Apr 7, 2020
`testReduceRandom` was bumping up against the serialization that I added in #54776. This makes it use random values that reduce in ways that don't cause the randomized serialization to fail.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
scripted_metricdid not work with cross cluster search because itassumed that you'd never perform a partial reduction, serialize the
results, and then perform a final reduction. That
serialized-after-partial-reduction step was broken.
This is also required to support #54758.