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

Metric to report time spent fetching and analyzing segments #14752

Merged
merged 11 commits into from
Aug 8, 2023

Conversation

suneet-s
Copy link
Contributor

@suneet-s suneet-s commented Aug 4, 2023

Description

Add a new metric that is emitted by the compaction task that shows how much time it spent fetching and analyzing segments. When a datasource is very fragmented, this could be a large amount of time. This metric will be helpful to operators as they can monitor this to see if they would benefit from explicitly specifying the schema in the auto compaction / manual compaction config if their use case allows it.

Here is a sample of what the metric would look like

{"taskIngestionMode":"REPLACE_LEGACY","feed":"metrics","taskType":"compact","metric":"compact/segmentAnalyzer/fetchAndProcessMillis","service":"druid/middleManager","groupId":"coordinator-issued_compact_wikipedia_cgmonfmg_2023-08-04T00:35:38.106Z","host":"localhost:8100","version":"28.0.0-SNAPSHOT","value":78,"dataSource":"wikipedia","taskId":"coordinator-issued_compact_wikipedia_cgmonfmg_2023-08-04T00:35:38.106Z","timestamp":"2023-08-04T00:35:40.605Z"}

Release note

NEW: compact/segmentAnalyzer/fetchAndProcessMillis is now reported by compaction tasks to indicate how much time was spent fetching and processing segments to infer the schema

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • 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.

@@ -904,6 +928,8 @@ static class ExistingSegmentAnalyzer
// For processMetricsSpec:
private final Set<List<AggregatorFactory>> aggregatorFactoryLists = new HashSet<>();

private long processingTimeMillis = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed. I had thought perhaps a metric for all the time spent in this class would be interesting, but decided against implementing it as it would have resulted in a few new metrics.

docs/operations/metrics.md Outdated Show resolved Hide resolved
docs/operations/metrics.md Outdated Show resolved Hide resolved
docs/operations/metrics.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ektravel ektravel left a comment

Choose a reason for hiding this comment

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

Reviewed metrics.md and left some suggestions.

@suneet-s
Copy link
Contributor Author

suneet-s commented Aug 7, 2023

Thanks @ektravel and @zachjsh for the reviews

@suneet-s suneet-s merged commit 2af0ab2 into apache:master Aug 8, 2023
@suneet-s suneet-s deleted the compact-time branch August 8, 2023 01:32
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants