You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/28 06:53:21 UTC

[GitHub] [dolphinscheduler] ruanwenjun opened a new pull request, #10649: Validate master/worker config

ruanwenjun opened a new pull request, #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649

   ## Purpose of the pull request
   
   Add validation to master worker config
   
   ## Brief change log
   
   - Validate master worker config properties.
   - Change all time properties in config to Duration.
   
   
   


-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#discussion_r908176885


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterConfig.java:
##########
@@ -21,170 +21,108 @@
 import org.apache.dolphinscheduler.server.master.processor.queue.TaskExecuteRunnable;
 import org.apache.dolphinscheduler.server.master.runner.WorkflowExecuteRunnable;
 
-import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.stereotype.Component;
+import java.time.Duration;
 
-@Component
-@EnableConfigurationProperties
-@ConfigurationProperties("master")
-public class MasterConfig {
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.validation.Errors;
+import org.springframework.validation.Validator;
+import org.springframework.validation.annotation.Validated;
+
+import lombok.Data;
+
+@Data
+@Validated
+@Configuration
+@ConfigurationProperties(prefix = "master")
+public class MasterConfig implements Validator {
     /**
      * The master RPC server listen port.
      */
-    private int listenPort;
+    private int listenPort = 5678;
     /**
      * The max batch size used to fetch command from database.
      */
-    private int fetchCommandNum;
+    private int fetchCommandNum = 10;
     /**
      * The thread number used to prepare processInstance. This number shouldn't bigger than fetchCommandNum.
      */
-    private int preExecThreads;
+    private int preExecThreads = 10;
     /**
      * todo: We may need to split the process/task into different thread size.
      * The thread number used to handle processInstance and task event.
      * Will create two thread poll to execute {@link WorkflowExecuteRunnable} and {@link TaskExecuteRunnable}.
      */
-    private int execThreads;
+    private int execThreads = 10;
     /**
      * The task dispatch thread pool size.
      */
-    private int dispatchTaskNumber;
+    private int dispatchTaskNumber = 3;
     /**
      * Worker select strategy.
      */
-    private HostSelector hostSelector;
+    private HostSelector hostSelector = HostSelector.LOWER_WEIGHT;
     /**
      * Master heart beat task execute interval.
      */
-    private int heartbeatInterval;
+    private Duration heartbeatInterval = Duration.ofSeconds(10);
     /**
      * task submit max retry times.
      */
-    private int taskCommitRetryTimes;
+    private int taskCommitRetryTimes = 5;
     /**
-     * task submit retry interval/ms.
+     * task submit retry interval.
      */
-    private int taskCommitInterval;
+    private Duration taskCommitInterval = Duration.ofSeconds(1);
     /**
-     * state wheel check interval/ms, if this value is bigger, may increase the delay of task/processInstance.
+     * state wheel check interval, if this value is bigger, may increase the delay of task/processInstance.
      */
-    private int stateWheelInterval;
-    private double maxCpuLoadAvg;
-    private double reservedMemory;
-    private int failoverInterval;
-    private boolean killYarnJobWhenTaskFailover;
-
-    public int getListenPort() {
-        return listenPort;
-    }
-
-    public void setListenPort(int listenPort) {
-        this.listenPort = listenPort;
-    }
-
-    public int getFetchCommandNum() {
-        return fetchCommandNum;
-    }
-
-    public void setFetchCommandNum(int fetchCommandNum) {
-        this.fetchCommandNum = fetchCommandNum;
-    }
-
-    public int getPreExecThreads() {
-        return preExecThreads;
-    }
-
-    public void setPreExecThreads(int preExecThreads) {
-        this.preExecThreads = preExecThreads;
-    }
-
-    public int getExecThreads() {
-        return execThreads;
-    }
-
-    public void setExecThreads(int execThreads) {
-        this.execThreads = execThreads;
-    }
-
-    public int getDispatchTaskNumber() {
-        return dispatchTaskNumber;
-    }
-
-    public void setDispatchTaskNumber(int dispatchTaskNumber) {
-        this.dispatchTaskNumber = dispatchTaskNumber;
-    }
-
-    public HostSelector getHostSelector() {
-        return hostSelector;
-    }
-
-    public void setHostSelector(HostSelector hostSelector) {
-        this.hostSelector = hostSelector;
-    }
-
-    public int getHeartbeatInterval() {
-        return heartbeatInterval;
-    }
-
-    public void setHeartbeatInterval(int heartbeatInterval) {
-        this.heartbeatInterval = heartbeatInterval;
-    }
-
-    public int getTaskCommitRetryTimes() {
-        return taskCommitRetryTimes;
-    }
-
-    public void setTaskCommitRetryTimes(int taskCommitRetryTimes) {
-        this.taskCommitRetryTimes = taskCommitRetryTimes;
-    }
-
-    public int getTaskCommitInterval() {
-        return taskCommitInterval;
-    }
-
-    public void setTaskCommitInterval(int taskCommitInterval) {
-        this.taskCommitInterval = taskCommitInterval;
-    }
-
-    public int getStateWheelInterval() {
-        return stateWheelInterval;
-    }
-
-    public void setStateWheelInterval(int stateWheelInterval) {
-        this.stateWheelInterval = stateWheelInterval;
-    }
-
-    public double getMaxCpuLoadAvg() {
-        return maxCpuLoadAvg > 0 ? maxCpuLoadAvg : Runtime.getRuntime().availableProcessors() * 2;
-    }
-
-    public void setMaxCpuLoadAvg(double maxCpuLoadAvg) {
-        this.maxCpuLoadAvg = maxCpuLoadAvg;
-    }
-
-    public double getReservedMemory() {
-        return reservedMemory;
-    }
-
-    public void setReservedMemory(double reservedMemory) {
-        this.reservedMemory = reservedMemory;
-    }
-
-    public int getFailoverInterval() {
-        return failoverInterval;
-    }
-
-    public void setFailoverInterval(int failoverInterval) {
-        this.failoverInterval = failoverInterval;
-    }
-
-    public boolean isKillYarnJobWhenTaskFailover() {
-        return killYarnJobWhenTaskFailover;
-    }
-
-    public void setKillYarnJobWhenTaskFailover(boolean killYarnJobWhenTaskFailover) {
-        this.killYarnJobWhenTaskFailover = killYarnJobWhenTaskFailover;
+    private Duration stateWheelInterval = Duration.ofMillis(5);
+    private double maxCpuLoadAvg = -1;
+    private double reservedMemory = 0.3;
+    private Duration failoverInterval = Duration.ofMinutes(10);
+    private boolean killYarnJobWhenTaskFailover = true;
+
+    @Override
+    public boolean supports(Class<?> clazz) {
+        return MasterConfig.class.isAssignableFrom(clazz);
+    }
+
+    @Override
+    public void validate(Object target, Errors errors) {
+        MasterConfig masterConfig = (MasterConfig) target;
+        if (masterConfig.getListenPort() <= 0) {
+            errors.rejectValue("listen-port", null, "is invalidated");
+        }
+        if (masterConfig.getFetchCommandNum() <= 0) {
+            errors.rejectValue("fetch-command-num", null, "should be a positive value");
+        }
+        if (masterConfig.getPreExecThreads() <= 0) {
+            errors.rejectValue("per-exec-threads", null, "should be a positive value");
+        }
+        if (masterConfig.getExecThreads() <= 0) {
+            errors.rejectValue("exec-threads", null, "should be a positive value");
+        }
+        if (masterConfig.getDispatchTaskNumber() <= 0) {
+            errors.rejectValue("dispatch-task-number", null, "should be a positive value");
+        }
+        if (masterConfig.getHeartbeatInterval().toMillis() < 0) {
+            errors.rejectValue("heartbeat-interval", null, "should be a valid duration");
+        }
+        if (masterConfig.getTaskCommitRetryTimes() <= 0) {
+            errors.rejectValue("task-commit-retry-times", null, "should better a positive value");

Review Comment:
   ```suggestion
               errors.rejectValue("task-commit-retry-times", null, "should be a positive value");
   ```



-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#discussion_r908248507


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterConfig.java:
##########
@@ -17,174 +17,87 @@
 
 package org.apache.dolphinscheduler.server.master.config;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import org.apache.dolphinscheduler.server.master.dispatch.host.assign.HostSelector;
 import org.apache.dolphinscheduler.server.master.processor.queue.TaskExecuteRunnable;
 import org.apache.dolphinscheduler.server.master.runner.WorkflowExecuteRunnable;
 
+import java.time.Duration;
+
+import javax.annotation.PostConstruct;
+
 import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.stereotype.Component;
+import org.springframework.context.annotation.Configuration;
+
+import lombok.Data;
 
-@Component
-@EnableConfigurationProperties
-@ConfigurationProperties("master")
+@Data
+@Configuration
+@ConfigurationProperties(prefix = "master")
 public class MasterConfig {
     /**
      * The master RPC server listen port.
      */
-    private int listenPort;
+    private int listenPort = 5678;
     /**
      * The max batch size used to fetch command from database.
      */
-    private int fetchCommandNum;
+    private int fetchCommandNum = 10;
     /**
      * The thread number used to prepare processInstance. This number shouldn't bigger than fetchCommandNum.
      */
-    private int preExecThreads;
+    private int preExecThreads = 10;
     /**
      * todo: We may need to split the process/task into different thread size.
      * The thread number used to handle processInstance and task event.
      * Will create two thread poll to execute {@link WorkflowExecuteRunnable} and {@link TaskExecuteRunnable}.
      */
-    private int execThreads;
+    private int execThreads = 10;
     /**
      * The task dispatch thread pool size.
      */
-    private int dispatchTaskNumber;
+    private int dispatchTaskNumber = 3;
     /**
      * Worker select strategy.
      */
-    private HostSelector hostSelector;
+    private HostSelector hostSelector = HostSelector.LOWER_WEIGHT;
     /**
      * Master heart beat task execute interval.
      */
-    private int heartbeatInterval;
+    private Duration heartbeatInterval = Duration.ofSeconds(10);
     /**
      * task submit max retry times.
      */
-    private int taskCommitRetryTimes;
+    private int taskCommitRetryTimes = 5;
     /**
-     * task submit retry interval/ms.
+     * task submit retry interval.
      */
-    private int taskCommitInterval;
+    private Duration taskCommitInterval = Duration.ofSeconds(1);
     /**
-     * state wheel check interval/ms, if this value is bigger, may increase the delay of task/processInstance.
+     * state wheel check interval, if this value is bigger, may increase the delay of task/processInstance.
      */
-    private int stateWheelInterval;
-    private double maxCpuLoadAvg;
-    private double reservedMemory;
-    private int failoverInterval;
-    private boolean killYarnJobWhenTaskFailover;
-
-    public int getListenPort() {
-        return listenPort;
-    }
-
-    public void setListenPort(int listenPort) {
-        this.listenPort = listenPort;
-    }
-
-    public int getFetchCommandNum() {
-        return fetchCommandNum;
-    }
-
-    public void setFetchCommandNum(int fetchCommandNum) {
-        this.fetchCommandNum = fetchCommandNum;
-    }
-
-    public int getPreExecThreads() {
-        return preExecThreads;
-    }
-
-    public void setPreExecThreads(int preExecThreads) {
-        this.preExecThreads = preExecThreads;
-    }
-
-    public int getExecThreads() {
-        return execThreads;
-    }
-
-    public void setExecThreads(int execThreads) {
-        this.execThreads = execThreads;
-    }
-
-    public int getDispatchTaskNumber() {
-        return dispatchTaskNumber;
-    }
-
-    public void setDispatchTaskNumber(int dispatchTaskNumber) {
-        this.dispatchTaskNumber = dispatchTaskNumber;
-    }
-
-    public HostSelector getHostSelector() {
-        return hostSelector;
-    }
-
-    public void setHostSelector(HostSelector hostSelector) {
-        this.hostSelector = hostSelector;
-    }
-
-    public int getHeartbeatInterval() {
-        return heartbeatInterval;
-    }
-
-    public void setHeartbeatInterval(int heartbeatInterval) {
-        this.heartbeatInterval = heartbeatInterval;
-    }
-
-    public int getTaskCommitRetryTimes() {
-        return taskCommitRetryTimes;
-    }
-
-    public void setTaskCommitRetryTimes(int taskCommitRetryTimes) {
-        this.taskCommitRetryTimes = taskCommitRetryTimes;
-    }
-
-    public int getTaskCommitInterval() {
-        return taskCommitInterval;
-    }
-
-    public void setTaskCommitInterval(int taskCommitInterval) {
-        this.taskCommitInterval = taskCommitInterval;
-    }
-
-    public int getStateWheelInterval() {
-        return stateWheelInterval;
-    }
-
-    public void setStateWheelInterval(int stateWheelInterval) {
-        this.stateWheelInterval = stateWheelInterval;
-    }
-
-    public double getMaxCpuLoadAvg() {
-        return maxCpuLoadAvg > 0 ? maxCpuLoadAvg : Runtime.getRuntime().availableProcessors() * 2;
-    }
-
-    public void setMaxCpuLoadAvg(double maxCpuLoadAvg) {
-        this.maxCpuLoadAvg = maxCpuLoadAvg;
-    }
-
-    public double getReservedMemory() {
-        return reservedMemory;
-    }
-
-    public void setReservedMemory(double reservedMemory) {
-        this.reservedMemory = reservedMemory;
-    }
-
-    public int getFailoverInterval() {
-        return failoverInterval;
-    }
-
-    public void setFailoverInterval(int failoverInterval) {
-        this.failoverInterval = failoverInterval;
-    }
-
-    public boolean isKillYarnJobWhenTaskFailover() {
-        return killYarnJobWhenTaskFailover;
-    }
-
-    public void setKillYarnJobWhenTaskFailover(boolean killYarnJobWhenTaskFailover) {
-        this.killYarnJobWhenTaskFailover = killYarnJobWhenTaskFailover;
+    private Duration stateWheelInterval = Duration.ofMillis(5);
+    private double maxCpuLoadAvg = -1;
+    private double reservedMemory = 0.3;
+    private Duration failoverInterval = Duration.ofMinutes(10);
+    private boolean killYarnJobWhenTaskFailover = true;
+
+    @PostConstruct
+    public void validate() {

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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#discussion_r908110486


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterConfig.java:
##########
@@ -17,174 +17,87 @@
 
 package org.apache.dolphinscheduler.server.master.config;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import org.apache.dolphinscheduler.server.master.dispatch.host.assign.HostSelector;
 import org.apache.dolphinscheduler.server.master.processor.queue.TaskExecuteRunnable;
 import org.apache.dolphinscheduler.server.master.runner.WorkflowExecuteRunnable;
 
+import java.time.Duration;
+
+import javax.annotation.PostConstruct;
+
 import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.stereotype.Component;
+import org.springframework.context.annotation.Configuration;
+
+import lombok.Data;
 
-@Component
-@EnableConfigurationProperties
-@ConfigurationProperties("master")
+@Data
+@Configuration
+@ConfigurationProperties(prefix = "master")
 public class MasterConfig {
     /**
      * The master RPC server listen port.
      */
-    private int listenPort;
+    private int listenPort = 5678;
     /**
      * The max batch size used to fetch command from database.
      */
-    private int fetchCommandNum;
+    private int fetchCommandNum = 10;
     /**
      * The thread number used to prepare processInstance. This number shouldn't bigger than fetchCommandNum.
      */
-    private int preExecThreads;
+    private int preExecThreads = 10;
     /**
      * todo: We may need to split the process/task into different thread size.
      * The thread number used to handle processInstance and task event.
      * Will create two thread poll to execute {@link WorkflowExecuteRunnable} and {@link TaskExecuteRunnable}.
      */
-    private int execThreads;
+    private int execThreads = 10;
     /**
      * The task dispatch thread pool size.
      */
-    private int dispatchTaskNumber;
+    private int dispatchTaskNumber = 3;
     /**
      * Worker select strategy.
      */
-    private HostSelector hostSelector;
+    private HostSelector hostSelector = HostSelector.LOWER_WEIGHT;
     /**
      * Master heart beat task execute interval.
      */
-    private int heartbeatInterval;
+    private Duration heartbeatInterval = Duration.ofSeconds(10);
     /**
      * task submit max retry times.
      */
-    private int taskCommitRetryTimes;
+    private int taskCommitRetryTimes = 5;
     /**
-     * task submit retry interval/ms.
+     * task submit retry interval.
      */
-    private int taskCommitInterval;
+    private Duration taskCommitInterval = Duration.ofSeconds(1);
     /**
-     * state wheel check interval/ms, if this value is bigger, may increase the delay of task/processInstance.
+     * state wheel check interval, if this value is bigger, may increase the delay of task/processInstance.
      */
-    private int stateWheelInterval;
-    private double maxCpuLoadAvg;
-    private double reservedMemory;
-    private int failoverInterval;
-    private boolean killYarnJobWhenTaskFailover;
-
-    public int getListenPort() {
-        return listenPort;
-    }
-
-    public void setListenPort(int listenPort) {
-        this.listenPort = listenPort;
-    }
-
-    public int getFetchCommandNum() {
-        return fetchCommandNum;
-    }
-
-    public void setFetchCommandNum(int fetchCommandNum) {
-        this.fetchCommandNum = fetchCommandNum;
-    }
-
-    public int getPreExecThreads() {
-        return preExecThreads;
-    }
-
-    public void setPreExecThreads(int preExecThreads) {
-        this.preExecThreads = preExecThreads;
-    }
-
-    public int getExecThreads() {
-        return execThreads;
-    }
-
-    public void setExecThreads(int execThreads) {
-        this.execThreads = execThreads;
-    }
-
-    public int getDispatchTaskNumber() {
-        return dispatchTaskNumber;
-    }
-
-    public void setDispatchTaskNumber(int dispatchTaskNumber) {
-        this.dispatchTaskNumber = dispatchTaskNumber;
-    }
-
-    public HostSelector getHostSelector() {
-        return hostSelector;
-    }
-
-    public void setHostSelector(HostSelector hostSelector) {
-        this.hostSelector = hostSelector;
-    }
-
-    public int getHeartbeatInterval() {
-        return heartbeatInterval;
-    }
-
-    public void setHeartbeatInterval(int heartbeatInterval) {
-        this.heartbeatInterval = heartbeatInterval;
-    }
-
-    public int getTaskCommitRetryTimes() {
-        return taskCommitRetryTimes;
-    }
-
-    public void setTaskCommitRetryTimes(int taskCommitRetryTimes) {
-        this.taskCommitRetryTimes = taskCommitRetryTimes;
-    }
-
-    public int getTaskCommitInterval() {
-        return taskCommitInterval;
-    }
-
-    public void setTaskCommitInterval(int taskCommitInterval) {
-        this.taskCommitInterval = taskCommitInterval;
-    }
-
-    public int getStateWheelInterval() {
-        return stateWheelInterval;
-    }
-
-    public void setStateWheelInterval(int stateWheelInterval) {
-        this.stateWheelInterval = stateWheelInterval;
-    }
-
-    public double getMaxCpuLoadAvg() {
-        return maxCpuLoadAvg > 0 ? maxCpuLoadAvg : Runtime.getRuntime().availableProcessors() * 2;
-    }
-
-    public void setMaxCpuLoadAvg(double maxCpuLoadAvg) {
-        this.maxCpuLoadAvg = maxCpuLoadAvg;
-    }
-
-    public double getReservedMemory() {
-        return reservedMemory;
-    }
-
-    public void setReservedMemory(double reservedMemory) {
-        this.reservedMemory = reservedMemory;
-    }
-
-    public int getFailoverInterval() {
-        return failoverInterval;
-    }
-
-    public void setFailoverInterval(int failoverInterval) {
-        this.failoverInterval = failoverInterval;
-    }
-
-    public boolean isKillYarnJobWhenTaskFailover() {
-        return killYarnJobWhenTaskFailover;
-    }
-
-    public void setKillYarnJobWhenTaskFailover(boolean killYarnJobWhenTaskFailover) {
-        this.killYarnJobWhenTaskFailover = killYarnJobWhenTaskFailover;
+    private Duration stateWheelInterval = Duration.ofMillis(5);
+    private double maxCpuLoadAvg = -1;
+    private double reservedMemory = 0.3;
+    private Duration failoverInterval = Duration.ofMinutes(10);
+    private boolean killYarnJobWhenTaskFailover = true;
+
+    @PostConstruct
+    public void validate() {

Review Comment:
   Can you implement the validation logic via implementing `org.springframework.validation.Validator` interface and add annotation `org.springframework.validation.annotation.Validated` to this class? That's what Spring propose to validate properties



-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#issuecomment-1168507461

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10649)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL)
   
   [![14.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '14.8%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_coverage&view=list) [14.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] kezhenxu94 merged pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649


-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#issuecomment-1168507162

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10649)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10649&resolved=false&types=CODE_SMELL)
   
   [![14.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '14.8%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_coverage&view=list) [14.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10649&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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@dolphinscheduler.apache.org

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


[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #10649: Validate master/worker config

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #10649:
URL: https://github.com/apache/dolphinscheduler/pull/10649#discussion_r908262251


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/config/MasterConfig.java:
##########
@@ -21,170 +21,108 @@
 import org.apache.dolphinscheduler.server.master.processor.queue.TaskExecuteRunnable;
 import org.apache.dolphinscheduler.server.master.runner.WorkflowExecuteRunnable;
 
-import org.springframework.boot.context.properties.ConfigurationProperties;
-import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.stereotype.Component;
+import java.time.Duration;
 
-@Component
-@EnableConfigurationProperties
-@ConfigurationProperties("master")
-public class MasterConfig {
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.validation.Errors;
+import org.springframework.validation.Validator;
+import org.springframework.validation.annotation.Validated;
+
+import lombok.Data;
+
+@Data
+@Validated
+@Configuration
+@ConfigurationProperties(prefix = "master")
+public class MasterConfig implements Validator {
     /**
      * The master RPC server listen port.
      */
-    private int listenPort;
+    private int listenPort = 5678;
     /**
      * The max batch size used to fetch command from database.
      */
-    private int fetchCommandNum;
+    private int fetchCommandNum = 10;
     /**
      * The thread number used to prepare processInstance. This number shouldn't bigger than fetchCommandNum.
      */
-    private int preExecThreads;
+    private int preExecThreads = 10;
     /**
      * todo: We may need to split the process/task into different thread size.
      * The thread number used to handle processInstance and task event.
      * Will create two thread poll to execute {@link WorkflowExecuteRunnable} and {@link TaskExecuteRunnable}.
      */
-    private int execThreads;
+    private int execThreads = 10;
     /**
      * The task dispatch thread pool size.
      */
-    private int dispatchTaskNumber;
+    private int dispatchTaskNumber = 3;
     /**
      * Worker select strategy.
      */
-    private HostSelector hostSelector;
+    private HostSelector hostSelector = HostSelector.LOWER_WEIGHT;
     /**
      * Master heart beat task execute interval.
      */
-    private int heartbeatInterval;
+    private Duration heartbeatInterval = Duration.ofSeconds(10);
     /**
      * task submit max retry times.
      */
-    private int taskCommitRetryTimes;
+    private int taskCommitRetryTimes = 5;
     /**
-     * task submit retry interval/ms.
+     * task submit retry interval.
      */
-    private int taskCommitInterval;
+    private Duration taskCommitInterval = Duration.ofSeconds(1);
     /**
-     * state wheel check interval/ms, if this value is bigger, may increase the delay of task/processInstance.
+     * state wheel check interval, if this value is bigger, may increase the delay of task/processInstance.
      */
-    private int stateWheelInterval;
-    private double maxCpuLoadAvg;
-    private double reservedMemory;
-    private int failoverInterval;
-    private boolean killYarnJobWhenTaskFailover;
-
-    public int getListenPort() {
-        return listenPort;
-    }
-
-    public void setListenPort(int listenPort) {
-        this.listenPort = listenPort;
-    }
-
-    public int getFetchCommandNum() {
-        return fetchCommandNum;
-    }
-
-    public void setFetchCommandNum(int fetchCommandNum) {
-        this.fetchCommandNum = fetchCommandNum;
-    }
-
-    public int getPreExecThreads() {
-        return preExecThreads;
-    }
-
-    public void setPreExecThreads(int preExecThreads) {
-        this.preExecThreads = preExecThreads;
-    }
-
-    public int getExecThreads() {
-        return execThreads;
-    }
-
-    public void setExecThreads(int execThreads) {
-        this.execThreads = execThreads;
-    }
-
-    public int getDispatchTaskNumber() {
-        return dispatchTaskNumber;
-    }
-
-    public void setDispatchTaskNumber(int dispatchTaskNumber) {
-        this.dispatchTaskNumber = dispatchTaskNumber;
-    }
-
-    public HostSelector getHostSelector() {
-        return hostSelector;
-    }
-
-    public void setHostSelector(HostSelector hostSelector) {
-        this.hostSelector = hostSelector;
-    }
-
-    public int getHeartbeatInterval() {
-        return heartbeatInterval;
-    }
-
-    public void setHeartbeatInterval(int heartbeatInterval) {
-        this.heartbeatInterval = heartbeatInterval;
-    }
-
-    public int getTaskCommitRetryTimes() {
-        return taskCommitRetryTimes;
-    }
-
-    public void setTaskCommitRetryTimes(int taskCommitRetryTimes) {
-        this.taskCommitRetryTimes = taskCommitRetryTimes;
-    }
-
-    public int getTaskCommitInterval() {
-        return taskCommitInterval;
-    }
-
-    public void setTaskCommitInterval(int taskCommitInterval) {
-        this.taskCommitInterval = taskCommitInterval;
-    }
-
-    public int getStateWheelInterval() {
-        return stateWheelInterval;
-    }
-
-    public void setStateWheelInterval(int stateWheelInterval) {
-        this.stateWheelInterval = stateWheelInterval;
-    }
-
-    public double getMaxCpuLoadAvg() {
-        return maxCpuLoadAvg > 0 ? maxCpuLoadAvg : Runtime.getRuntime().availableProcessors() * 2;
-    }
-
-    public void setMaxCpuLoadAvg(double maxCpuLoadAvg) {
-        this.maxCpuLoadAvg = maxCpuLoadAvg;
-    }
-
-    public double getReservedMemory() {
-        return reservedMemory;
-    }
-
-    public void setReservedMemory(double reservedMemory) {
-        this.reservedMemory = reservedMemory;
-    }
-
-    public int getFailoverInterval() {
-        return failoverInterval;
-    }
-
-    public void setFailoverInterval(int failoverInterval) {
-        this.failoverInterval = failoverInterval;
-    }
-
-    public boolean isKillYarnJobWhenTaskFailover() {
-        return killYarnJobWhenTaskFailover;
-    }
-
-    public void setKillYarnJobWhenTaskFailover(boolean killYarnJobWhenTaskFailover) {
-        this.killYarnJobWhenTaskFailover = killYarnJobWhenTaskFailover;
+    private Duration stateWheelInterval = Duration.ofMillis(5);
+    private double maxCpuLoadAvg = -1;
+    private double reservedMemory = 0.3;
+    private Duration failoverInterval = Duration.ofMinutes(10);
+    private boolean killYarnJobWhenTaskFailover = true;
+
+    @Override
+    public boolean supports(Class<?> clazz) {
+        return MasterConfig.class.isAssignableFrom(clazz);
+    }
+
+    @Override
+    public void validate(Object target, Errors errors) {
+        MasterConfig masterConfig = (MasterConfig) target;
+        if (masterConfig.getListenPort() <= 0) {
+            errors.rejectValue("listen-port", null, "is invalidated");
+        }
+        if (masterConfig.getFetchCommandNum() <= 0) {
+            errors.rejectValue("fetch-command-num", null, "should be a positive value");
+        }
+        if (masterConfig.getPreExecThreads() <= 0) {
+            errors.rejectValue("per-exec-threads", null, "should be a positive value");
+        }
+        if (masterConfig.getExecThreads() <= 0) {
+            errors.rejectValue("exec-threads", null, "should be a positive value");
+        }
+        if (masterConfig.getDispatchTaskNumber() <= 0) {
+            errors.rejectValue("dispatch-task-number", null, "should be a positive value");
+        }
+        if (masterConfig.getHeartbeatInterval().toMillis() < 0) {
+            errors.rejectValue("heartbeat-interval", null, "should be a valid duration");
+        }
+        if (masterConfig.getTaskCommitRetryTimes() <= 0) {
+            errors.rejectValue("task-commit-retry-times", null, "should better a positive value");

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@dolphinscheduler.apache.org

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