-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix NPE on FieldStats with mixed cluster on version pre/post 5.2 #22688
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
Conversation
In 5.2 the FieldStats API can return null min/max values. These values cannot be deserialized by a node with version pre 5.2 so if this node is pick to coordinate a FieldStats request in a mixed cluster an NPE can be thrown. This change prevents the NPE by removing the non serializable FieldStats object directly in the field stats shard request. The filtered fields will not be present in the response when a node pre 5.2 acts as a coordinating node.
s1monw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM I left one question / suggestion
| } | ||
| } else { | ||
| if (hasMinMax == false) { | ||
| throw new IllegalArgumentException("cannot serialize null min/max fieldstats in a mixed-cluster " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this only happens if we mess something up right? I wonder if we should make it an assertion instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes if it happens it's a bug, I'll make it an assertion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
|
Thansk @s1monw. Now merging to the 5.x and 5.2 |
) * Fix NPE on FieldStats with mixed cluster on version pre/post 5.2 In 5.2 the FieldStats API can return null min/max values. These values cannot be deserialized by a node with version pre 5.2 so if this node is pick to coordinate a FieldStats request in a mixed cluster an NPE can be thrown. This change prevents the NPE by removing the non serializable FieldStats object directly in the field stats shard request. The filtered fields will not be present in the response when a node pre 5.2 acts as a coordinating node.
) * Fix NPE on FieldStats with mixed cluster on version pre/post 5.2 In 5.2 the FieldStats API can return null min/max values. These values cannot be deserialized by a node with version pre 5.2 so if this node is pick to coordinate a FieldStats request in a mixed cluster an NPE can be thrown. This change prevents the NPE by removing the non serializable FieldStats object directly in the field stats shard request. The filtered fields will not be present in the response when a node pre 5.2 acts as a coordinating node.
* master: (117 commits) Add missing serialization BWC for disk usage estimates Expose disk usage estimates in nodes stats S3 repository: Deprecate specifying credentials through env vars, sys props, and remove profile files (elastic#22567) Fix Eclipse project generation Fix deprecation logging for lenient booleans Remove @Header we no longer need Make lexer abstract [Docs] Remove outdated info about enabling/disabling doc_values (elastic#22694) Move lexer hacks to EnhancedPainlessLexer Fix incorrect args order passed to createAggregator Improve painless's javadocs Add TestWithDependenciesPlugin to build (elastic#22646) Add parsing from xContent to SearchProfileShardResults and nested classes (elastic#22649) Add unit tests for FiltersAggregator (elastic#22678) Don't register search response listener in transport clients unmute FieldStatsIntegrationIT.testGeoPointNotIndexed, fix already pushed Mute FieldStatsIntegrationIT.testGeoPointNotIndexed, for now Painless: Add augmentation to string for base 64 (elastic#22665) Fix NPE on FieldStats with mixed cluster on version pre/post 5.2 (elastic#22688) Add parsing methods for UpdateResponse (elastic#22586) ...
In 5.2 the FieldStats API can return null min/max values.
These values cannot be deserialized by a node with version pre 5.2 so if this node
is pick to coordinate a FieldStats request in a mixed cluster an NPE can be thrown.
This change prevents the NPE by removing the non serializable FieldStats object directly in the field stats shard request.
The filtered fields will not be present in the response when a node pre 5.2 acts as a coordinating node.