[GitHub] [helix] jiajunwang commented on a change in pull request #1011: Add CRUD endpoints to TaskDriver for configurable thread pool size support

[GitBox <gi...@apache.org>]

jiajunwang commented on a change in pull request #1011:
URL: https://github.com/apache/helix/pull/1011#discussion_r425498891



##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskDriver.java
##########
@@ -1137,4 +1140,89 @@ private void validateZKNodeLimitation(int newConfigNodeCount) {
           "Cannot create more workflows or jobs because there are already too many items created in the path CONFIGS.");
     }
   }
+
+  /**
+   * Get the target task thread pool size of an instance, a value that's used to construct the task
+   * thread pool and is created by users.
+   * @param instanceName - name of the instance
+   * @return the target task thread pool size of the instance
+   */
+  public int getTargetTaskThreadPoolSize(String instanceName) {
+    InstanceConfig instanceConfig =

Review comment:
       can you create a private method with these common lines to avoid duplication? 

##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskDriver.java
##########
@@ -1137,4 +1140,89 @@ private void validateZKNodeLimitation(int newConfigNodeCount) {
           "Cannot create more workflows or jobs because there are already too many items created in the path CONFIGS.");
     }
   }
+
+  /**
+   * Get the target task thread pool size of an instance, a value that's used to construct the task
+   * thread pool and is created by users.
+   * @param instanceName - name of the instance
+   * @return the target task thread pool size of the instance
+   */
+  public int getTargetTaskThreadPoolSize(String instanceName) {
+    InstanceConfig instanceConfig =
+        _accessor.getProperty(_accessor.keyBuilder().instanceConfig(instanceName));
+    if (instanceConfig == null) {
+      throw new IllegalArgumentException(
+          "Failed to find InstanceConfig with provided instance name " + instanceName);
+    }
+
+    return instanceConfig.getTargetTaskThreadPoolSize();
+  }
+
+  /**
+   * Set the target task thread pool size of an instance. The target task thread pool size goes to
+   * InstanceConfig, and is used to construct the task thread pool. The construction of thread pool
+   * requires JVM restart after the target value has been set.
+   * @param instanceName - name of the instance
+   * @param targetTaskThreadPoolSize - the target task thread pool size of the instance
+   */
+  public void setTargetTaskThreadPoolSize(String instanceName, int targetTaskThreadPoolSize) {
+    InstanceConfig instanceConfig =
+        _accessor.getProperty(_accessor.keyBuilder().instanceConfig(instanceName));
+    if (instanceConfig == null) {
+      throw new IllegalArgumentException(
+          "Failed to find InstanceConfig with provided instance name " + instanceName + "!");
+    }
+
+    instanceConfig.setTargetTaskThreadPoolSize(targetTaskThreadPoolSize);
+  }
+
+  /**
+   * Get the global target task thread pool size of the cluster, a value that's used to construct
+   * task thread pools for the cluster's instances and is created by users.
+   * @return the global target task thread pool size of the cluster
+   */
+  public int getGlobalTargetTaskThreadPoolSize() {
+    ClusterConfig clusterConfig = _accessor.getProperty(_accessor.keyBuilder().clusterConfig());

Review comment:
       Same here. Duplicate code.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org