You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/03/18 22:39:05 UTC
[GitHub] [druid] maytasm opened a new pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
maytasm opened a new pull request #12352:
URL: https://github.com/apache/druid/pull/12352
Duties in Indexing group (such as Auto Compaction) does not report metrics
### Description
Duties emit metrics via the EmitClusterStatsAndMetrics class which is run as the last duty in the group. However, this duty is only added to the HistoricalManagementDuties group and not other groups such as IndexingServiceDuties group. This PR changes the following:
- EmitClusterStatsAndMetrics is automatically added to all duty group via the constructor of the DutiesRunnable (if not already exist)
- EmitClusterStatsAndMetrics is change to check for list of duty and group name to only emits relevant metrics.
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] 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 or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] 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](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm merged pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
Posted by GitBox <gi...@apache.org>.
maytasm merged pull request #12352:
URL: https://github.com/apache/druid/pull/12352
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #12352:
URL: https://github.com/apache/druid/pull/12352#issuecomment-1075629055
Added `Release Notes` since there will be new metrics emitted by services other than the historical.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #12352:
URL: https://github.com/apache/druid/pull/12352#discussion_r831317481
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
##########
@@ -841,7 +840,14 @@ CompactSegments initializeCompactSegmentsDuty()
protected DutiesRunnable(List<? extends CoordinatorDuty> duties, final int startingLeaderCounter, String alias)
{
- this.duties = duties;
+ if (duties.stream().noneMatch(duty -> duty instanceof EmitClusterStatsAndMetrics)) {
+ List<CoordinatorDuty> allDuties = new ArrayList<>(duties);
+ allDuties.add(new EmitClusterStatsAndMetrics(DruidCoordinator.this, alias, duties));
+ this.duties = allDuties;
+ } else {
+ this.duties = duties;
+ }
+ //Auto add emit
Review comment:
What does this comment mean?
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/duty/EmitClusterStatsAndMetrics.java
##########
@@ -133,6 +142,21 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params)
CoordinatorStats stats = params.getCoordinatorStats();
ServiceEmitter emitter = params.getEmitter();
+ if (DruidCoordinator.HISTORICAL_MANAGEMENT_DUTIES_DUTY_GROUP.equals(groupName)) {
+ emitStatsForHistoricalManagementDuties(cluster, stats, emitter, params);
+ }
+ if (dutyList.stream().anyMatch(duty -> duty instanceof CompactSegments)) {
Review comment:
Instead of passing dutiesList as a parameter to the constructor, I'd recommend passing in a boolean parameter, to make it clear that this duty doesn't deal with any of the other duties. Otherwise LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #12352:
URL: https://github.com/apache/druid/pull/12352#discussion_r832684678
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
##########
@@ -841,7 +840,14 @@ CompactSegments initializeCompactSegmentsDuty()
protected DutiesRunnable(List<? extends CoordinatorDuty> duties, final int startingLeaderCounter, String alias)
{
- this.duties = duties;
+ if (duties.stream().noneMatch(duty -> duty instanceof EmitClusterStatsAndMetrics)) {
+ List<CoordinatorDuty> allDuties = new ArrayList<>(duties);
+ allDuties.add(new EmitClusterStatsAndMetrics(DruidCoordinator.this, alias, duties));
+ this.duties = allDuties;
+ } else {
+ this.duties = duties;
+ }
+ //Auto add emit
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #12352: Duties in Indexing group (such as Auto Compaction) does not report metrics
Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #12352:
URL: https://github.com/apache/druid/pull/12352#discussion_r832689786
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/duty/EmitClusterStatsAndMetrics.java
##########
@@ -133,6 +142,21 @@ public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params)
CoordinatorStats stats = params.getCoordinatorStats();
ServiceEmitter emitter = params.getEmitter();
+ if (DruidCoordinator.HISTORICAL_MANAGEMENT_DUTIES_DUTY_GROUP.equals(groupName)) {
+ emitStatsForHistoricalManagementDuties(cluster, stats, emitter, params);
+ }
+ if (dutyList.stream().anyMatch(duty -> duty instanceof CompactSegments)) {
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org