Skip to content
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

Expose rule stats in QueryStats #16365

Merged
merged 1 commit into from
Mar 31, 2023
Merged

Conversation

Dith3r
Copy link
Member

@Dith3r Dith3r commented Mar 3, 2023

Description

Fixes #2578.

Expose optimizer rule statistics per query in QueryInfo JSON. The number of rules exposed could be adjusted using the query.reported-rule-stats-limit configuration parameter.

Example output:

    "optimizerRulesSummaries" : [ {
    "rule" : "io.trino.sql.planner.iterative.rule.PushPredicateIntoTableScan",
    "invocations" : 10,
    "applications" : 1,
    "totalTime" : 999,
    "failures" : 0
  },
   (...)] 

Release notes

( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Mar 3, 2023
@Dith3r Dith3r requested a review from sopel39 March 3, 2023 14:59
@github-actions github-actions bot added the iceberg Iceberg connector label Mar 3, 2023
@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from 5f0ecc9 to aba4078 Compare March 6, 2023 13:02
@github-actions github-actions bot added the docs label Mar 6, 2023
@Dith3r Dith3r force-pushed the ke/rules-stats branch 3 times, most recently from b55f35f to 33176e1 Compare March 7, 2023 07:39
@Dith3r Dith3r marked this pull request as ready for review March 7, 2023 07:39
@Dith3r
Copy link
Member Author

Dith3r commented Mar 7, 2023

Failure due #16383

@Dith3r Dith3r requested a review from lukasz-stec March 7, 2023 10:37
@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from ebc5d2c to ddaab70 Compare March 8, 2023 07:23
@Dith3r
Copy link
Member Author

Dith3r commented Mar 8, 2023

Failed due #16406

@Dith3r Dith3r force-pushed the ke/rules-stats branch 3 times, most recently from 411f3b4 to e08c90c Compare March 8, 2023 15:12
@sopel39
Copy link
Member

sopel39 commented Mar 8, 2023

Are the rules ordered by total time?

@Dith3r
Copy link
Member Author

Dith3r commented Mar 8, 2023

Yes, rules are ordered by totalTime in descending order.

@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from 32c82aa to bf062f3 Compare March 9, 2023 15:31
@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from c093910 to 0278cfa Compare March 27, 2023 15:21
@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from a5f32a4 to f72f91f Compare March 28, 2023 07:05
@Dith3r Dith3r requested a review from martint March 28, 2023 10:23
@Dith3r Dith3r requested a review from martint March 29, 2023 08:05
@Dith3r Dith3r force-pushed the ke/rules-stats branch 2 times, most recently from a07fc2d to 6d825a0 Compare March 29, 2023 13:15
Expose optimizer rule statistics per query in QueryInfo JSON. The number of rules exposed could be
adjusted using the `query.reported-rule-stats-limit` configuration parameter.
@martint martint merged commit e1ed6e9 into trinodb:master Mar 31, 2023
@github-actions github-actions bot added this to the 412 milestone Mar 31, 2023
@colebow
Copy link
Member

colebow commented Apr 4, 2023

Because this includes a new configuration property, it needs documentation. Would you be able to do a follow-up PR for that, or would you prefer that I handle it?

cc @martint

@sopel39
Copy link
Member

sopel39 commented Apr 4, 2023

cc @Dith3r ^^

@Dith3r
Copy link
Member Author

Dith3r commented Apr 5, 2023

In one of the patchsets I've removed documentation as we agreed that this is very internal thing (query JSON is only affected). If we want to add documentation, I can create new PR for it - please only confirm if this is needed.

@findepi
Copy link
Member

findepi commented Apr 11, 2023

follow-up -- #16960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Expose rule stats in QueryStats
7 participants