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

Add metrics for number of segments generated per task in MSQ #14980

Merged
merged 4 commits into from
Sep 25, 2023

Conversation

YongGang
Copy link
Contributor

@YongGang YongGang commented Sep 13, 2023

Description

Report ingest/tombstones/count and ingest/segments/count metrics in MSQ when finish publishing segments.

Following are two examples:

{"taskIngestionMode":"NONE","feed":"metrics","taskType":"query_controller","metric":"ingest/tombstones/count","service":"druid/indexer","groupId":"query-481a0c2d-1f04-4a5b-bc30-f343240850d3","host":"localhost:8091","version":"","value":0,"dataSource":"wikipedia","taskId":"query-481a0c2d-1f04-4a5b-bc30-f343240850d3","timestamp":"2023-09-14T17:01:47.951Z"}

{"taskIngestionMode":"NONE","feed":"metrics","taskType":"query_controller","metric":"ingest/segments/count","service":"druid/indexer","groupId":"query-481a0c2d-1f04-4a5b-bc30-f343240850d3","host":"localhost:8091","version":"","value":1,"dataSource":"wikipedia","taskId":"query-481a0c2d-1f04-4a5b-bc30-f343240850d3","timestamp":"2023-09-14T17:01:47.954Z"}

Release note

Add ingest/tombstones/count and ingest/segments/count metrics in MSQ.


Key changed/added classes in this PR
  • In ControllerImpl report the new metrics

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Copy link
Contributor

@LakshSingla LakshSingla left a comment

Choose a reason for hiding this comment

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

  1. Is there a way to distinguish between the metrics generated by MSQ and the native ingestion? Since we are selectively adding a few metrics, it would be nice to be able to filter by the mode of ingestion.
  2. We'd wanna document this in the MSQ docs and/or the metrics docs somehow

@kfaraz
Copy link
Contributor

kfaraz commented Sep 14, 2023

Is there a way to distinguish between the metrics generated by MSQ and the native ingestion? Since we are selectively adding a few metrics, it would be nice to be able to filter by the mode of ingestion.

@LakshSingla , I suppose you could filter by the type of task that has reported that metric.

…id/msq/exec/ControllerImpl.java

Co-authored-by: Kashif Faraz <[email protected]>
@YongGang
Copy link
Contributor Author

Is there a way to distinguish between the metrics generated by MSQ and the native ingestion? Since we are selectively adding a few metrics, it would be nice to be able to filter by the mode of ingestion.

@LakshSingla , I suppose you could filter by the type of task that has reported that metric.

yup, can use task type to distinguish. I added two examples of the metrics generated in the PR description.

@YongGang
Copy link
Contributor Author

Hi @LakshSingla @kfaraz , does this change look good to you?

@github-actions github-actions bot added Area - Metrics/Event Emitting Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 and removed Area - Querying labels Sep 18, 2023
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

LGTM

@YongGang
Copy link
Contributor Author

Hi @LakshSingla , does this change look good to you? The main purpose for these two metrics is for alerting: when a task creates way many segments, it will make the cluster unstable, we want to be alerted in this case.

@YongGang
Copy link
Contributor Author

fyi @cryptoe

@LakshSingla LakshSingla merged commit 7301e60 into apache:master Sep 25, 2023
@LakshSingla
Copy link
Contributor

Merging the PR, cc @cryptoe if we require doc changes for the same.

@LakshSingla LakshSingla added this to the 28.0 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area - Batch Ingestion Area - Metrics/Event Emitting Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants