Skip to content

Introduce query/timeout/count metric#10567

Merged
himanshug merged 3 commits intoapache:masterfrom
a2l007:timeout_metric
Nov 20, 2020
Merged

Introduce query/timeout/count metric#10567
himanshug merged 3 commits intoapache:masterfrom
a2l007:timeout_metric

Conversation

@a2l007
Copy link
Copy Markdown
Contributor

@a2l007 a2l007 commented Nov 9, 2020

Follow-up PR to #10464.

This PR adds a new query metric query/timeout/count that represents the number of timed out queries during the emission period. Timed out query counts were previously clubbed under query/interrupted/count. Having a separate metric for timed out queries can make it a little easier for cluster operators to diagnose query issues better.

Marking this as Incompatible as query/interrupted/count no longer represents timed out query counts and user defined alerting/reports may need to be adjusted to factor in query/timeout/count along with query/interrupted/count.

It would be useful to add metricsMonitors to integration tests so that it provides better testing for PRs such as this.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

@capistrant
Copy link
Copy Markdown
Contributor

This looks good at initial glance. I'll test and submit a review tomorrow when I have some spare time

Copy link
Copy Markdown
Member

@nishantmonu51 nishantmonu51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 , LGTM.

Copy link
Copy Markdown
Contributor

@capistrant capistrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

reviewed as well as pulled down code and ran locally. example metric:

{"feed":"metrics","timestamp":"2020-11-12T18:53:55.392Z","service":"druid/broker","host":"broker:8082","version":"0.21.0-SNAPSHOT","metric":"query/timeout/count","value":12}

@himanshug himanshug merged commit 111b431 into apache:master Nov 20, 2020
@jihoonson jihoonson added this to the 0.21.0 milestone Jan 4, 2021
JulianJaffePinterest pushed a commit to JulianJaffePinterest/druid that referenced this pull request Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants