Improve QueryInfo and StatementStats performance#17495
Merged
pettyjamesm merged 3 commits intoprestodb:masterfrom Apr 22, 2022
Merged
Improve QueryInfo and StatementStats performance#17495pettyjamesm merged 3 commits intoprestodb:masterfrom
pettyjamesm merged 3 commits intoprestodb:masterfrom
Conversation
b77d447 to
f6e6968
Compare
Avoids extra allocations and copies associated with Immutable collection building and removes an additional recursive child stage traversal associated with producing StatementStats and StageStats in the critical path of producing query results responses.
Refactors QueryInfo to consolidate usages of isCompleteInfo() and isFinalInfo() into a single method: isFinalInfo(). The removed isCompleteInfo field and method lacked a @JsonProperty annotation and was therefore not being serialized anyway, and the isFinalInfo method which was annotated as a @JsonProperty for serialization had no corresponding constructor field and so also would not survive a serialize / deserialize round-trip and required re-building a list of StageInfo on each call, which is unnecessary overhead. The consolidation of the two related fields ensures that the work of determining whether a QueryInfo is "final" is done once while producing the QueryInfo object and successfully round-trips when serialized an deserialized.
Refactors the QueryInfo and QueryStats creation logic to call getAllStages once, instead of four times as was being done before this change. Each call to getAllStages will build an ImmutableList with each StageInfo which can be relatively expensive for queries that have a large number of stages. This can be significant, since QueryInfo creation occurs on each batch of query results fetched through the coordinator.
f6e6968 to
96f7804
Compare
highker
approved these changes
Apr 22, 2022
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.
Similar changes extracted from trinodb/trino#11580
Refactors logic that creates
QueryInfoandStatementStatsto reduce overhead and avoid repeated work. This code sits in the critical path of the coordinator producing query result responses.Each commit has a more specific description of the exact change being made. At a high level, they are:
QueryInfo/StageInfotoStatementStats/StageStatsQueryInfoto consolidateisCompletedInfo()andisFinalInfo()into a single methodStageInfo.getAllStagesso that child stages only need to be flattened into a single list once, instead of four times as was the case before this change