Use separate executor to handle task updates in TaskQueue by kfaraz · Pull Request #14533 · apache/druid

kfaraz · 2023-07-06T11:22:44Z

Description

TaskQueue.notifyStatus is often a heavy call as it performs the following operations:

Update task status in metadata DB
Update task locks in metadata DB
Request (synchronously) the task runner to shutdown the completed task
Clean up in-memory data structures

This method can often be slow and can cause worker sync / task runners to slow down.

Changes

Run task completion callbacks in a separate executor to handle task completion updates
Add config to handle the number of threads of the new callback executor
Add metrics to monitor number of processed and queued items
Use CoordinatorRunStats to collect and report metrics. This class has been used as is for now but will later be renamed and repurposed across all Druid services.
Add TaskCountStatsProvider.getStats() which deprecates the other getXXXTaskCount methods.
There are still other paths that can invoke notifyStatus, but those need not be moved to the new executor as they are synchronous on purpose.

Release note

Add new config druid.indexer.queue.taskCompleteHandlerNumThreads which controls the number of threads used by the Overlord TaskQueue to handle task completion updates received from the workers.
Add new metrics task/status/queue/count, task/status/updated/count

This PR has:

abhishekagarwal87 · 2023-07-06T11:39:36Z

    }
  }

+  public CoordinatorRunStats getQueueStats()


Hmm. These are not coordinator run stats.

Yes, I have added a comment in the description of this PR about repurposing the CoordinatorRunStats to be generic DruidStats but that will be tackled in a separate PR.

This class is a nice encapsulation to collect and report stats across multiple dimensions and is thread-safe too.

abhishekagarwal87 · 2023-07-06T11:44:19Z

+        // The onSuccess and onFailure should be lightweight enough
+        // so that the executor giving the statusFuture is not blocked


Suggested change

// The onSuccess and onFailure should be lightweight enough

// so that the executor giving the statusFuture is not blocked

// The onSuccess and onFailure should be lightweight enough

// so that the executor giving the statusFuture is not blocked

// We use the direct executor here so we can track in-flight status updates.

// so the metrics for in-flight status gets updated immediately.

Simplified this comment, added the rest of the info in the javadoc of this method as it seemed like a better place for detailed richer content.

kfaraz · 2023-07-07T15:13:24Z

Thanks for the review, @abhishekagarwal87 !

vtlim · 2023-07-14T00:46:51Z

@kfaraz In the release notes, should task/status/handled/count really be task/status/updated/count? From

druid/indexing-service/src/main/java/org/apache/druid/indexing/overlord/Stats.java

Lines 31 to 34 in 95ca430

    
           public static final CoordinatorStat STATUS_UPDATES_IN_QUEUE 
        
               = CoordinatorStat.toDebugAndEmit("queuedStatusUpdates", "task/status/queue/count"); 
        
           public static final CoordinatorStat HANDLED_STATUS_UPDATES 
        
               = CoordinatorStat.toDebugAndEmit("handledStatusUpdates", "task/status/updated/count");

kfaraz · 2023-07-14T03:02:06Z

Yes, @vtlim , thanks for catching this!

Description: `TaskQueue.notifyStatus` is often a heavy call as it performs the following operations: - Update task status in metadata DB - Update task locks in metadata DB - Request (synchronously) the task runner to shutdown the completed task - Clean up in-memory data structures This method can often be slow and can cause worker sync / task runners to slow down. Main changes: - Run task completion callbacks in a separate executor to handle task completion updates - Add new config `druid.indexer.queue.taskCompleteHandlerNumThreads` - Add metrics to monitor number of processed and queued items - There are still other paths that can invoke `notifyStatus`, but those need not be moved to the new executor as they are synchronous on purpose. Other changes: - Add new metrics `task/status/queue/count`, `task/status/handled/count` - Add `TaskCountStatsProvider.getStats()` which deprecates the other `getXXXTaskCount` methods. - Use `CoordinatorRunStats` to collect and report metrics. This class has been used as is for now but will later be renamed and repurposed to use across all Druid services.

acherla · 2023-08-09T11:55:55Z

@kfaraz I am wondering if we can instead default this to a percentage of CPU cores on the machine type instead of defaulting it to a value of 5 if the value is not present for taskCompleteHandlerNumThreads? Maybe max 30% of available Runtime.getRuntime().availableProcessors() or 5? This would reduce the amount of configuration a user would have to do on initial setup and id consider this feature a pretty big component for the stability of the overlord when a large number of task statuses are updated.

this.taskCompleteHandlerNumThreads = Configs.valueOrDefault(taskCompleteHandlerNumThreads, 5);

maytasm · 2025-05-26T08:16:47Z

@kfaraz Any reason these metrics and config property are not in the docs? I don't think this is internal config and can see users setting these if they are high number of indexing tasks

kfaraz · 2025-05-26T08:54:17Z

@maytasm , these were originally meant to be an experimental config and metrics. So we didn't document them originally.

Have you encountered cases where the default value of 5 was inadequate?
What value did you end up choosing in that case?

I have personally never had to reconfigure this value or needed to look at the these metrics for any real debugging.
But since they are there, we might as well document them. I can create a PR for that.

I can also implement the suggestion in this comment #14533 (comment) to reduce the config required.

Let me know what you think.

maytasm · 2025-05-26T19:55:08Z

@kfaraz Our cluster has about 2500 kafka indexing tasks and 3000 compaction related indexing tasks running. When many tasks roll over at the same time, we saw some issues with the overlord not processing the task updates fast enough. We had instances where it was so slow that we exceeded the completionTimeout on the kafka indexing tasks, causing them to fail. Before trying out this change, we manage it by configuring the stopTaskCount on each supervisor and using taskDuration that are prime numbers (to minimize tasks rolling at the same time). The default value of 5 was inadequate for us. We set the config to be 1/3 of the number of CPU we have (which is about 30) and life has been great. task/status/queue/count has been stable and low (no spikes), we haven't had issue with completionTimeout, and didn't need to tune stopTaskCount+taskDuration.

kfaraz · 2025-05-27T01:44:05Z

Sounds good, thanks for the details, @maytasm ! I will create a PR to update the docs.

Description: `TaskQueue.notifyStatus` is often a heavy call as it performs the following operations: - Update task status in metadata DB - Update task locks in metadata DB - Request (synchronously) the task runner to shutdown the completed task - Clean up in-memory data structures This method can often be slow and can cause worker sync / task runners to slow down. Main changes: - Run task completion callbacks in a separate executor to handle task completion updates - Add new config `druid.indexer.queue.taskCompleteHandlerNumThreads` - Add metrics to monitor number of processed and queued items - There are still other paths that can invoke `notifyStatus`, but those need not be moved to the new executor as they are synchronous on purpose. Other changes: - Add new metrics `task/status/queue/count`, `task/status/handled/count` - Add `TaskCountStatsProvider.getStats()` which deprecates the other `getXXXTaskCount` methods. - Use `CoordinatorRunStats` to collect and report metrics. This class has been used as is for now but will later be renamed and repurposed to use across all Druid services.

Use separate executor to handle task updates in TaskQueue

c2115dc

kfaraz added Performance Area - Ingestion labels Jul 6, 2023

Add license and fix checkstyle

4f82bb7

abhishekagarwal87 reviewed Jul 6, 2023

View reviewed changes

kfaraz added 4 commits July 6, 2023 17:35

Add empty instance of CoordinatorRunStats

65c280c

Fix tests and coverage

1953d65

Increase coco for TaskMaster

26c6690

Fix thread name of TaskQueue callback thread

ba5f117

abhishekagarwal87 approved these changes Jul 7, 2023

View reviewed changes

kfaraz merged commit 40d0dc9 into apache:master Jul 7, 2023

kfaraz deleted the task_callback_exec branch July 7, 2023 15:13

abhishekagarwal87 added this to the 27.0 milestone Jul 19, 2023

AmatyaAvadhanula mentioned this pull request Aug 6, 2023

[DRAFT] 27.0.0 release notes #14761

Closed

abhishekagarwal87 mentioned this pull request Aug 16, 2023

Community roadmap 2023 #14157

Open

maytasm mentioned this pull request Apr 30, 2025

Kafka ingestion lag spikes up whenever tasks are rolling #11414

Open

Navigation Menu

Repository navigation

Use separate executor to handle task updates in TaskQueue - #14533

Use separate executor to handle task updates in TaskQueue#14533
kfaraz merged 6 commits into
apache:masterfrom
kfaraz:task_callback_exec

kfaraz commented Jul 6, 2023 •

edited

Loading

Uh oh!

abhishekagarwal87 Jul 6, 2023

Uh oh!

kfaraz Jul 6, 2023

Uh oh!

abhishekagarwal87 Jul 6, 2023

Uh oh!

kfaraz Jul 6, 2023

Uh oh!

kfaraz commented Jul 7, 2023

Uh oh!

vtlim commented Jul 14, 2023

Uh oh!

kfaraz commented Jul 14, 2023

Uh oh!

acherla commented Aug 9, 2023 •

edited

Loading

Uh oh!

maytasm commented May 26, 2025

Uh oh!

kfaraz commented May 26, 2025

Uh oh!

maytasm commented May 26, 2025 •

edited

Loading

Uh oh!

kfaraz commented May 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		// The onSuccess and onFailure should be lightweight enough
		// so that the executor giving the statusFuture is not blocked

Conversation

kfaraz commented Jul 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Release note

Uh oh!

abhishekagarwal87 Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

kfaraz Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

abhishekagarwal87 Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

kfaraz Jul 6, 2023

Choose a reason for hiding this comment

Uh oh!

kfaraz commented Jul 7, 2023

Uh oh!

vtlim commented Jul 14, 2023

Uh oh!

kfaraz commented Jul 14, 2023

Uh oh!

acherla commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maytasm commented May 26, 2025

Uh oh!

kfaraz commented May 26, 2025

Uh oh!

maytasm commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kfaraz commented May 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kfaraz commented Jul 6, 2023 •

edited

Loading

acherla commented Aug 9, 2023 •

edited

Loading

maytasm commented May 26, 2025 •

edited

Loading