Use thread priorities. (aka set `nice` values for background-like tasks) #984

drcrallen · 2014-12-19T22:46:08Z

Defaults the thread priority to java.util.Thread.NORM_PRIORITY in io.druid.indexing.common.task.AbstractTask
Each exec service has its own Task Factory which is assigned a priority for spawned task. Therefore each priority class has a unique exec service
Added taskPriority priority as a task context parameter. <0 means low, 0 means take default, >0 means high. It is up to any particular implementation to determine how to handle these numbers
The following options should be used to enable linux thread scheduling:
- -XX:+UseThreadPriorities
- -XX:ThreadPriorityPolicy=42
AbstractTask - Removed unneded @JsonIgnore on priority
Added priority to RealtimePlumber executors. All sub-executors (non query runners) get Thread.MIN_PRIORITY
Add persistThreadPriority and mergeThreadPriority to realtime tuning config

Running with -XX:+UseThreadPriorities -XX:ThreadPriorityPolicy=1 with root privileges enables this
Running with -XX:+UseThreadPriorities -XX:ThreadPriorityPolicy=1 WITHOUT root privileges DOES NOT enables this. And will give the warning Java HotSpot(TM) 64-Bit Server VM warning: -XX:ThreadPriorityPolicy requires root privilege on Linux
Running with -XX:+UseThreadPriorities does NOT enable this
Running with -XX:+UseThreadPriorities -XX:ThreadPriorityPolicy=42 REGARDLESS of root privileges enables this

This PR is to hopefully help when a task is running on a node that also needs to service queries (notably a realtime node doing persisting and merging)

The impact on query time can be seen in the image below.

As you can see, the maximum query time, and the query time during the long merge process, seem unaffected by this patch. While the persists are accumulating before the merge, the query time linearly increases (assumed proportional to the data intake, though I have nothing to back up that assumption). The max value reached before the persist-n-merge portion is also too close to make any sort of claim. What is obvious, however, is that the first part is completed notably faster.

It is worth noting that CGroups override nice values, so this setting should not interfere with CGroup resources for co-tenant workloads.

fjy · 2015-03-10T22:58:07Z

indexing-service/src/main/java/io/druid/indexing/common/task/Task.java

@@ -54,8 +54,33 @@
 })
 public interface Task
 {
+  public static enum Priority
+  {
+    // NOTE: Setting negative nice values on linux sysetems (priority > Thread.NORM_PRIORITY) requires running


fjy · 2015-03-26T22:29:08Z

I think we need to test this PR a bit more to understand if the possible benefits justify the code complexity. I also think we should make sure that persists will not be starved out with these changes.

drcrallen · 2015-03-31T17:39:32Z

I did simple tests, and when running a local firehose (so just firing off events as fast as they can be parsed from TPCH file) then the realtime index task runs in about 20% LESS TIME (good) to get to the first persist-n-merge when the persist and the merge thread pools are LOW priority as compared to NORM. Executing any queries on the nodes tends to have no measurable impact on the total ingestion time on my local machine (<50% cpu usage for ingest, persist, merge, and query all combined).

I did not wait for the persist-n-merge to complete. The reason is that looking at the thread breakout, the persist and realtime index task (plumber) are the ones that run concurrently, with the merge task running all by its lonesome.

This is on my Mac though, so mileage on Linux may vary since the schedulers are not exactly the same.

xvrl · 2015-05-01T17:16:02Z

indexing-service/src/main/java/io/druid/indexing/overlord/ForkingTaskRunner.java

@@ -204,6 +204,9 @@ public TaskStatus call()
                              command.add(String.format("-Ddruid.host=%s", childHost));
                              command.add(String.format("-Ddruid.port=%d", childPort));

+                              command.add("-XX:+UseThreadPriorities");
+                              command.add("-XX:ThreadPriorityPolicy=42");


can we add a link to the documentation explaining this parameter

if this is some weird undocumented behavior in the vm that we rely on, I would prefer we simply put a note in the documentation about this and let users set the parameter themselves in the forking task runner vm parameter.

https://docs.oracle.com/cd/E15289_01/doc.40/e15062/optionxx.htm#BABGBFHF

Without setting the command line flags the behavior is disabled at the JVM level

xvrl · 2015-05-01T19:19:30Z

common/src/main/java/io/druid/concurrent/Execs.java

@@ -35,24 +37,52 @@
 */
 public class Execs
 {
-  public static ExecutorService singleThreaded(String nameFormat)
+  private static int nullToNormal(@Nullable Integer priority)


I think it would greatly simplify the code if we removed this method and just defaulted to null, only setting the priority if it is actually set to something, rather than assuming the default is normal.

the default IS normal.

And the code would actually be more complex since you couldn't have the cases with no priority call the cases with priority with NULL values

the default is technically the priority of the creating thread.
Even if in our case this might be normal all the time, I think it will make things less cluttered to not not define a default everywhere and leave it null

drcrallen · 2015-09-04T23:17:21Z

@nishantmonu51 I updated this PR to use your Task context

nishantmonu51 · 2015-09-09T03:16:55Z

common/src/main/java/io/druid/concurrent/TaskPriority.java

+public class TaskPriority
+{
+  // The task context key to grab the task priority from
+  public static final String CONTEXT_KEY = "taskPriority";


With the changes in #1679,
there will be two context values "priority" and "taskPriority" which looks similar but have different semantics.
Can we rename this to taskThreadPriority to be more explicit ?

sure, sounds good

renaming to backgroundThreadPriority would also be fine.

nishantmonu51 · 2015-09-09T03:17:53Z

we should also add docs for the new configs.

thedrow · 2015-10-18T10:00:02Z

This PR needs to be rebased.

drcrallen · 2015-10-18T15:13:47Z

@thedrow yes, and I'm supposed to have a meeting with @himanshug and some other folks on task priority.

drcrallen · 2015-10-18T15:14:01Z

Then I'll worry about rebasing against current master

thedrow · 2015-12-01T15:38:00Z

@drcrallen Any news on this one? It should be useful if merged.

drcrallen · 2015-12-18T23:36:14Z

@nishantmonu51 how would you feel about leaving it as an advanced feature for now?

nishantmonu51 · 2016-01-12T17:37:06Z

👍

drcrallen · 2016-01-20T19:20:00Z

Conflicts were trivial. All around extra config options

fjy · 2016-01-20T19:23:53Z

server/src/main/java/io/druid/segment/indexing/RealtimeTuningConfig.java

-      @JsonProperty("buildV9Directly") Boolean buildV9Directly
+      @JsonProperty("buildV9Directly") Boolean buildV9Directly,
+      @JsonProperty("persistThreadPriority") int persistThreadPriority,
+      @JsonProperty("mergeThreadPriority") int mergeThreadPriority


docs and good default values required

Defaults revert to prior behavior and do not set thread priority.

@JsonIgnore

* Defaults the thread priority to java.util.Thread.NORM_PRIORITY in io.druid.indexing.common.task.AbstractTask * Each exec service has its own Task Factory which is assigned a priority for spawned task. Therefore each priority class has a unique exec service * Added priority to tasks as taskPriority in the task context. <0 means low, 0 means take default, >0 means high. It is up to any particular implementation to determine how to handle these numbers * Add options to ForkingTaskRunner * Add "-XX:+UseThreadPriorities" default option * Add "-XX:ThreadPriorityPolicy=42" default option * AbstractTask - Removed unneded @JsonIgnore on priority * Added priority to RealtimePlumber executors. All sub-executors (non query runners) get Thread.MIN_PRIORITY * Add persistThreadPriority and mergeThreadPriority to realtime tuning config

fjy · 2016-01-21T02:41:17Z

👍

Use thread priorities. (aka set `nice` values for background-like tasks)

…ation

drcrallen closed this Jan 21, 2015

drcrallen reopened this Jan 21, 2015

xvrl added this to the 0.7.1 milestone Jan 29, 2015

xvrl added the Performance label Jan 29, 2015

fjy force-pushed the master branch 2 times, most recently from 8b0ec82 to d05032b Compare February 1, 2015 04:57

drcrallen force-pushed the thread-priority-rebase branch from 7171d10 to 89dca15 Compare February 23, 2015 16:52

drcrallen closed this Feb 27, 2015

drcrallen reopened this Feb 27, 2015

fjy reviewed Mar 10, 2015
View reviewed changes

drcrallen force-pushed the thread-priority-rebase branch 2 times, most recently from f04d7e3 to 4a3d9cf Compare March 10, 2015 23:11

drcrallen closed this Mar 10, 2015

drcrallen reopened this Mar 10, 2015

xvrl modified the milestone: 0.7.1 Mar 26, 2015

drcrallen closed this Apr 17, 2015

drcrallen reopened this Apr 17, 2015

xvrl reviewed May 1, 2015
View reviewed changes

drcrallen force-pushed the thread-priority-rebase branch 3 times, most recently from 63f027f to 0027b91 Compare May 1, 2015 18:04

xvrl reviewed May 1, 2015
View reviewed changes

drcrallen force-pushed the thread-priority-rebase branch from afe4a04 to 109d92a Compare September 4, 2015 23:16

drcrallen force-pushed the thread-priority-rebase branch from 109d92a to 57adf0a Compare September 4, 2015 23:23

nishantmonu51 reviewed Sep 9, 2015
View reviewed changes

drcrallen mentioned this pull request Sep 9, 2015

Priority based task locking #1679

Closed

drcrallen added this to the 0.9.0 milestone Dec 1, 2015

drcrallen force-pushed the thread-priority-rebase branch from 57adf0a to 5166ebe Compare December 18, 2015 23:35

drcrallen closed this Dec 19, 2015

drcrallen reopened this Dec 19, 2015

drcrallen force-pushed the thread-priority-rebase branch 2 times, most recently from f8d7e5e to 1dfe80f Compare January 12, 2016 17:30

drcrallen force-pushed the thread-priority-rebase branch from 1dfe80f to 8a43d77 Compare January 20, 2016 19:19

fjy reviewed Jan 20, 2016
View reviewed changes

drcrallen force-pushed the thread-priority-rebase branch from 8a43d77 to 2e1d6aa Compare January 20, 2016 22:00

nishantmonu51 added a commit that referenced this pull request Jan 21, 2016

Merge pull request #984 from drcrallen/thread-priority-rebase

dcb7830

Use thread priorities. (aka set `nice` values for background-like tasks)

nishantmonu51 merged commit dcb7830 into apache:master Jan 21, 2016

fjy mentioned this pull request Feb 5, 2016

druid-0.9.0 release notes #2404

Closed

drcrallen deleted the thread-priority-rebase branch January 12, 2017 01:23

seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Jan 10, 2020

apache#984 Allow extension handler in query server for Avatica integr…

8c057fa

…ation

vladak mentioned this pull request Apr 14, 2021

Suggester feature rebuilding its indexes causing high load on the system oracle/opengrok#3522

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use thread priorities. (aka set `nice` values for background-like tasks) #984

Use thread priorities. (aka set `nice` values for background-like tasks) #984

drcrallen commented Dec 19, 2014

fjy Mar 10, 2015

drcrallen Mar 10, 2015

fjy commented Mar 26, 2015

drcrallen commented Mar 31, 2015

xvrl May 1, 2015

xvrl May 1, 2015

drcrallen May 1, 2015

drcrallen May 1, 2015

xvrl May 1, 2015

drcrallen May 1, 2015

drcrallen May 1, 2015

xvrl May 1, 2015

drcrallen commented Sep 4, 2015

nishantmonu51 Sep 9, 2015

drcrallen Sep 9, 2015

nishantmonu51 Sep 25, 2015

nishantmonu51 commented Sep 9, 2015

thedrow commented Oct 18, 2015

drcrallen commented Oct 18, 2015

drcrallen commented Oct 18, 2015

thedrow commented Dec 1, 2015

drcrallen commented Dec 18, 2015

nishantmonu51 commented Jan 12, 2016

drcrallen commented Jan 20, 2016

fjy Jan 20, 2016

drcrallen Jan 20, 2016

drcrallen Jan 20, 2016

fjy commented Jan 21, 2016

Use thread priorities. (aka set nice values for background-like tasks) #984

Use thread priorities. (aka set nice values for background-like tasks) #984

Conversation

drcrallen commented Dec 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjy commented Mar 26, 2015

drcrallen commented Mar 31, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drcrallen commented Sep 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nishantmonu51 commented Sep 9, 2015

thedrow commented Oct 18, 2015

drcrallen commented Oct 18, 2015

drcrallen commented Oct 18, 2015

thedrow commented Dec 1, 2015

drcrallen commented Dec 18, 2015

nishantmonu51 commented Jan 12, 2016

drcrallen commented Jan 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjy commented Jan 21, 2016

Use thread priorities. (aka set `nice` values for background-like tasks) #984

Use thread priorities. (aka set `nice` values for background-like tasks) #984