Fix profiling naming issues#27133
Merged
polyfractal merged 1 commit intoelastic:masterfrom Nov 6, 2017
Merged
Conversation
Some code-paths use anonymous classes (such as NonCollectingAggregator in terms agg), which messes up the display name of the profiler. If we encounter an anonymous class, we need to grab the super's name. Another naming issue was that ProfileAggs were not delegating to the wrapped agg's name for toString(), leading to ugly display. This PR also fixes up the profile documentation. Some of the examples were executing against empty indices, which shows different profile results than a populated index (and made for confusing examples). Finally, I switched the agg display names from the fully qualified name to the simple name, so that it's similar to how the query profiles work. Closes elastic#26405
jpountz
approved these changes
Oct 27, 2017
polyfractal
pushed a commit
that referenced
this pull request
Nov 6, 2017
Some code-paths use anonymous classes (such as NonCollectingAggregator in terms agg), which messes up the display name of the profiler. If we encounter an anonymous class, we need to grab the super's name. Another naming issue was that ProfileAggs were not delegating to the wrapped agg's name for toString(), leading to ugly display. This PR also fixes up the profile documentation. Some of the examples were executing against empty indices, which shows different profile results than a populated index (and made for confusing examples). Finally, I switched the agg display names from the fully qualified name to the simple name, so that it's similar to how the query profiles work. Closes #26405
jasontedor
added a commit
that referenced
this pull request
Nov 7, 2017
* master: (25 commits) Disable bwc tests in preparation of backporting #26931 TemplateUpgradeService should only run on the master (#27294) Die with dignity while merging Fix profiling naming issues (#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (#27283) Add an active Elasticsearch WordPress plugin link (#27279) Setting url parts as required to reflect the code base (#27263) keys in aggs percentiles need to be in quotes. (#26905) Align routing param type with search.json (#26958) Update to support bulk updates by query (#27172) Remove duplicated SnapshotStatus (#27276) add split index reference in indices.asciidoc Add ability to split shards (#26931) [Docs] Fix minor paragraph indentation error for multiple Indices params (#25535) Upgrade to Jackson 2.8.10 (#27230) Fix inconsistencies in the rest api specs for `tasks` (#27163) Adjust RestHighLevelClient method modifiers (#27238) Remove unused parameters in AnalysisRegistry (#27232) Add more information on `_failed_to_convert_` exception (#27034) ...
jasontedor
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
Nov 7, 2017
* ccr: (127 commits) Disable bwc tests in preparation of backporting elastic#26931 TemplateUpgradeService should only run on the master (elastic#27294) Die with dignity while merging Fix profiling naming issues (elastic#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (elastic#27283) Add an active Elasticsearch WordPress plugin link (elastic#27279) Setting url parts as required to reflect the code base (elastic#27263) keys in aggs percentiles need to be in quotes. (elastic#26905) Align routing param type with search.json (elastic#26958) Update to support bulk updates by query (elastic#27172) Remove duplicated SnapshotStatus (elastic#27276) add split index reference in indices.asciidoc Add ability to split shards (elastic#26931) [Docs] Fix minor paragraph indentation error for multiple Indices params (elastic#25535) Upgrade to Jackson 2.8.10 (elastic#27230) Fix inconsistencies in the rest api specs for `tasks` (elastic#27163) Adjust RestHighLevelClient method modifiers (elastic#27238) Remove unused parameters in AnalysisRegistry (elastic#27232) Add more information on `_failed_to_convert_` exception (elastic#27034) ...
jasontedor
added a commit
that referenced
this pull request
Nov 7, 2017
* 6.x: Update shrink's bwc version to 6.1.0 add split index reference in indices.asciidoc Add ability to split shards (#26931) TemplateUpgradeService should only run on the master (#27294) Die with dignity while merging Fix profiling naming issues (#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (#27283) Add an active Elasticsearch WordPress plugin link (#27279) Setting url parts as required to reflect the code base (#27263) keys in aggs percentiles need to be in quotes. (#26905) Align routing param type with search.json (#26958) Update to support bulk updates by query (#27172) Remove duplicated SnapshotStatus (#27276)
jasontedor
added a commit
to glefloch/elasticsearch
that referenced
this pull request
Nov 9, 2017
* master: (556 commits) Fix find remote when building BWC Remove colons from task and configuration names Add unreleased 5.6.5 version number testCreateSplitIndexToN: do not set `routing_partition_size` to >= `number_of_routing_shards` Snapshot/Restore: better handle incorrect chunk_size settings in FS repo (elastic#26844) Add limits for ngram and shingle settings (elastic#27211) (elastic#27318) Correct comment in index shard test Roll translog generation on primary promotion ObjectParser: Replace IllegalStateException with ParsingException (elastic#27302) scripted_metric _agg parameter disappears if params are provided (elastic#27159) Update discovery-ec2.asciidoc Update shrink's bwc version to 6.1.0 and enabled bwc tests Add limits for ngram and shingle settings (elastic#27211) Disable bwc tests in preparation of backporting elastic#26931 TemplateUpgradeService should only run on the master (elastic#27294) Die with dignity while merging Fix profiling naming issues (elastic#27133) Correctly encode warning headers Fixed references to Multi Index Syntax (elastic#27283) Add an active Elasticsearch WordPress plugin link (elastic#27279) ...
Member
|
@polyfractal This PR has the "backport pending" label but it appears to have been backported to all of the versions with labels on this PR? Can the label be removed now? |
Contributor
Author
|
@jasontedor I was thinking this should go into 6.0.1, not sure why I didn't add the 6.0.1 label though (probably just overlooked it). I'll add the label |
Member
|
Thanks, and this can be safely backported now that 6.0 is GA and then we can remove the |
Contributor
Author
|
Backported in 5990364, removed label. :) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Some code-paths use anonymous classes (such as
NonCollectingAggregatorin terms agg), which messes up the display name of the profiler. If we encounter an anonymous class, we need to grab the super's name.Another naming issue was that ProfileAggs were not delegating to the wrapped agg's name for
toString(). That meant some places were showing the class name instead of the user-defined name.This PR also fixes up the profile documentation. Some of the examples were executing against empty indices, which shows different profile results than a populated index (and made for confusing examples).
Finally, I switched the agg display names from the fully qualified name to the simple name, so that it's similar to how the query profiles work.
I don't think any of this is BWC breaking since it's just changing the display name from useless/ugly to useful... but happy to hear alternative thoughts. :)