Add indexer level task metrics to provide more visibility in the task distribution #15991

rbankar7 · 2024-02-28T04:37:31Z

Adds indexer level task metrics

Description

This PR adds indexer level task metrics-

"worker/task/running/count"
"worker/task/assigned/count"
"worker/task/completed/count"

These metrics will provide more visibility into the tasks distribution across indexers (We often see a task skew issue across indexers and with this issue it would be easier to catch the imbalance)

This PR has:

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitor.java

kfaraz

Thanks for your first PR, @rbankar7 ! I have left some suggestions.

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitor.java

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManager.java

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitor.java

kfaraz · 2024-02-28T07:53:47Z

...ing-service/src/test/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitorTest.java

+  @Before
+  public void setUp()
+  {
+    task = createMock(Task.class);


Use NoopTask instead of creating a mock.

kfaraz · 2024-02-28T07:54:33Z

...ing-service/src/test/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitorTest.java

+    taskDetails = createMock(WorkerTaskManager.TaskDetails.class);
+    EasyMock.expect(taskDetails.getDataSource()).andReturn("dummy_DS2");
+    EasyMock.replay(taskDetails);
+    taskAnnouncement = createMock(TaskAnnouncement.class);


Try to use concrete instances of TaskAnnouncement and TaskDetails (if needed) instead of mocks.

kfaraz · 2024-02-28T07:55:14Z

...ing-service/src/test/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitorTest.java

+    EasyMock.replay(taskAnnouncement);
+    taskRunner = createMock(TaskRunner.class);
+    taskConfig = createMock(TaskConfig.class);
+    overlordClient = createMock(OverlordClient.class);


Use NoopOverlordClient instead.

kfaraz

Thanks for the update, @rbankar7 .
I have left some comments that need to be addressed.
Please update the docs to add info about the new metrics.

server/src/main/java/org/apache/druid/server/metrics/WorkerTaskCountStatsMonitor.java

server/src/test/java/org/apache/druid/server/metrics/WorkerTaskCountStatsMonitorTest.java

server/src/main/java/org/apache/druid/server/metrics/IndexerTaskCountStatsProvider.java

server/src/main/java/org/apache/druid/server/metrics/WorkerTaskCountStatsProvider.java

server/src/main/java/org/apache/druid/server/metrics/WorkerTaskCountStatsMonitor.java

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManager.java

rbankar7 · 2024-03-07T09:32:17Z

Thanks for the update, @rbankar7 . I have left some comments that need to be addressed. Please update the docs to add info about the new metrics.

Thanks @kfaraz for the review
I have addressed the comments and updated the doc with the new metrics

kfaraz

Thanks for adding the new metric and docs, @rbankar7 !

kfaraz · 2024-03-07T15:10:06Z

server/src/main/java/org/apache/druid/server/metrics/IndexerTaskCountStatsProvider.java

+public interface IndexerTaskCountStatsProvider
+{
+  /**
+   * The number of running tasks on an indexer


Minor suggestion for the javadocs but this can be addressed later.

Suggested change

* The number of running tasks on an indexer

* Map from datasource name to the number of running tasks on the Indexer.

…indexerTaskCountStatsProvides

kfaraz · 2024-03-08T02:28:31Z

@rbankar7 , the checkstyle and WorkerTaskCountStatsMonitorTest are failing. Please take a look.

kfaraz · 2024-03-09T03:56:28Z

@rbankar7 , there are more checkstyle failures. To ensure that you catch all of them, please run the following on your local machine before you commit:

mvn checkstyle:checkstyle

rbankar7 · 2024-03-15T08:22:54Z

@kfaraz I fixed the issues and verified that there are no checkstyle errors
Thanks!

kfaraz · 2024-03-15T11:25:28Z

@rbankar7 , I ran the workflow. The testWorkerTaskCountStatsMonitorTest.testMonitorNotMiddleManager seems to be failing.

Error:  Errors: 
Error:  org.apache.druid.server.metrics.WorkerTaskCountStatsMonitorTest.testMonitorNotMiddleManager
Error:    Run 1: WorkerTaskCountStatsMonitorTest.testMonitorNotMiddleManager:288 » Configuration Guice configuration errors:

1) No implementation for org.apache.druid.server.metrics.IndexerTaskCountStatsProvider was bound.
  while locating org.apache.druid.server.metrics.IndexerTaskCountStatsProvider

The code coverage also seems insufficient for WorkerTaskManager.

rbankar7 · 2024-03-15T12:02:46Z

I ran the workflow. The testWorkerTaskCountStatsMonitorTest.testMonitorNotMiddleManager seems to be failing.

@kfaraz will fix it
Thanks

…nitorIndexer) and add tests for getting worker stats

rbankar7 · 2024-03-21T05:26:37Z

@kfaraz looks like build is passing now
could you please take a look
Thanks!

kfaraz · 2024-03-21T05:38:34Z

Thanks a lot for the fix, @rbankar7 !
Congrats on your first commit to Apache Druid! 🎉

… distribution (apache#15991) Changes: Add the following indexer level task metrics: - `worker/task/running/count` - `worker/task/assigned/count` - `worker/task/completed/count` These metrics will provide more visibility into the tasks distribution across indexers (We often see a task skew issue across indexers and with this issue it would be easier to catch the imbalance)

…re visibility (#191) * Add indexer level task metrics to provide more visibility in the task distribution (apache#15991) Changes: Add the following indexer level task metrics: - `worker/task/running/count` - `worker/task/assigned/count` - `worker/task/completed/count` These metrics will provide more visibility into the tasks distribution across indexers (We often see a task skew issue across indexers and with this issue it would be easier to catch the imbalance)

… distribution (apache#15991) (#192) Changes: Add the following indexer level task metrics: - `worker/task/running/count` - `worker/task/assigned/count` - `worker/task/completed/count` These metrics will provide more visibility into the tasks distribution across indexers (We often see a task skew issue across indexers and with this issue it would be easier to catch the imbalance)

Add indexer level task metrics

59b4e96

github-actions bot added Area - Metrics/Event Emitting Area - Ingestion labels Feb 28, 2024

rbankar7 marked this pull request as ready for review February 28, 2024 04:37

abhishekagarwal87 reviewed Feb 28, 2024

View reviewed changes

indexing-service/src/main/java/org/apache/druid/indexing/worker/WorkerTaskManagerMonitor.java Outdated Show resolved Hide resolved

kfaraz reviewed Feb 28, 2024

View reviewed changes

Address review comments

e2da519

kfaraz reviewed Mar 7, 2024

View reviewed changes

rbankar7 added 3 commits March 7, 2024 14:19

Address review comments

f14f10c

Add javaDoc for the IndexerTaskCountStatsProvider interface

893ce4f

Small typo fix and update docs to include new metrics

6948d5f

github-actions bot added the Area - Documentation label Mar 7, 2024

rbankar7 requested a review from kfaraz March 7, 2024 09:33

kfaraz approved these changes Mar 7, 2024

View reviewed changes

Address comment and bind the WorkerTaskManager implementation to the …

061aac0

…indexerTaskCountStatsProvides

Fix checkstyle errors and remove unused import

80c6f65

Fix checkstyle errors

928a1c5

Fix build (remove testMonitorNotMiddleManager as it is same as testMo…

051cc84

…nitorIndexer) and add tests for getting worker stats

kfaraz merged commit 3d8b0ff into apache:master Mar 21, 2024
85 checks passed

rbankar7 mentioned this pull request Mar 22, 2024

[OBSDATA-3335][Backport] Add indexer level task metrics to provide more visibility confluentinc/druid#191

Merged

10 tasks

rbankar7 mentioned this pull request Mar 22, 2024

[OBSDATA-3335][Backport] Add indexer level task metrics to provide more visibility confluentinc/druid#192

Merged

10 tasks

adarshsanjeev added this to the 30.0.0 milestone May 6, 2024

adarshsanjeev mentioned this pull request May 28, 2024

[DRAFT] 30.0.0 release notes #16505

Closed

jakubmatyszewski mentioned this pull request Aug 7, 2024

WorkerTaskCountStatsMonitor doesn't work in Druid 30.0.0 #16855

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add indexer level task metrics to provide more visibility in the task distribution #15991

Add indexer level task metrics to provide more visibility in the task distribution #15991

rbankar7 commented Feb 28, 2024 •

edited

Loading

kfaraz left a comment

kfaraz Feb 28, 2024

kfaraz Feb 28, 2024

kfaraz Feb 28, 2024

kfaraz left a comment

rbankar7 commented Mar 7, 2024

kfaraz left a comment

kfaraz Mar 7, 2024

kfaraz commented Mar 8, 2024

kfaraz commented Mar 9, 2024

rbankar7 commented Mar 15, 2024

kfaraz commented Mar 15, 2024

rbankar7 commented Mar 15, 2024 •

edited

Loading

rbankar7 commented Mar 21, 2024

kfaraz commented Mar 21, 2024

	* The number of running tasks on an indexer
	* Map from datasource name to the number of running tasks on the Indexer.

Add indexer level task metrics to provide more visibility in the task distribution #15991

Add indexer level task metrics to provide more visibility in the task distribution #15991

Conversation

rbankar7 commented Feb 28, 2024 • edited Loading

Description

kfaraz left a comment

Choose a reason for hiding this comment

kfaraz Feb 28, 2024

Choose a reason for hiding this comment

kfaraz Feb 28, 2024

Choose a reason for hiding this comment

kfaraz Feb 28, 2024

Choose a reason for hiding this comment

kfaraz left a comment

Choose a reason for hiding this comment

rbankar7 commented Mar 7, 2024

kfaraz left a comment

Choose a reason for hiding this comment

kfaraz Mar 7, 2024

Choose a reason for hiding this comment

kfaraz commented Mar 8, 2024

kfaraz commented Mar 9, 2024

rbankar7 commented Mar 15, 2024

kfaraz commented Mar 15, 2024

rbankar7 commented Mar 15, 2024 • edited Loading

rbankar7 commented Mar 21, 2024

kfaraz commented Mar 21, 2024

rbankar7 commented Feb 28, 2024 •

edited

Loading

rbankar7 commented Mar 15, 2024 •

edited

Loading