[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10631: [Bug] [Master] Worker failover will cause task cannot be failover

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

kezhenxu94 commented on code in PR #10631:
URL: https://github.com/apache/dolphinscheduler/pull/10631#discussion_r908080548


##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/OSUtils.java:
##########
@@ -460,23 +460,23 @@ public static int getProcessID() {
     }
 
     /**
-     * check memory and cpu usage
+     * Check memory and cpu usage is overload the given thredshod.
      *
-     * @param maxCpuloadAvg maxCpuloadAvg
+     * @param maxCpuLoadAvg  maxCpuLoadAvg
      * @param reservedMemory reservedMemory
-     * @return check memory and cpu usage
+     * @return True, if the cpu or memory exceed the given thredshod.
      */
-    public static Boolean checkResource(double maxCpuloadAvg, double reservedMemory) {
+    public static Boolean checkOverload(double maxCpuLoadAvg, double reservedMemory) {

Review Comment:
   ```suggestion
       public static boolean checkOverload(double maxCpuLoadAvg, double reservedMemory) {
   ```
   
   It'd be more clear if we can rename to something like `isOverloaded`



##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/OSUtils.java:
##########
@@ -460,23 +460,23 @@ public static int getProcessID() {
     }
 
     /**
-     * check memory and cpu usage
+     * Check memory and cpu usage is overload the given thredshod.
      *
-     * @param maxCpuloadAvg maxCpuloadAvg
+     * @param maxCpuLoadAvg  maxCpuLoadAvg
      * @param reservedMemory reservedMemory
-     * @return check memory and cpu usage
+     * @return True, if the cpu or memory exceed the given thredshod.
      */
-    public static Boolean checkResource(double maxCpuloadAvg, double reservedMemory) {
+    public static Boolean checkOverload(double maxCpuLoadAvg, double reservedMemory) {
         // system load average
         double loadAverage = loadAverage();
         // system available physical memory
         double availablePhysicalMemorySize = availablePhysicalMemorySize();
-        if (loadAverage > maxCpuloadAvg || availablePhysicalMemorySize < reservedMemory) {
-            logger.warn("current cpu load average {} is too high or available memory {}G is too low, under max.cpuload.avg={} and reserved.memory={}G",
-                    loadAverage, availablePhysicalMemorySize, maxCpuloadAvg, reservedMemory);
-            return false;
-        } else {
+        if (loadAverage > maxCpuLoadAvg || availablePhysicalMemorySize < reservedMemory) {
+            logger.warn("Current cpu load average {} is too high or available memory {}G is too low, under max.cpuLoad.avg={} and reserved.memory={}G",
+                loadAverage, availablePhysicalMemorySize, maxCpuLoadAvg, reservedMemory);
             return true;
+        } else {
+            return false;

Review Comment:
   ```suggestion
           }
           return false;
   ```



##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/service/FailoverService.java:
##########
@@ -67,15 +71,20 @@ public class FailoverService {
     private final MasterConfig masterConfig;
     private final ProcessService processService;
     private final WorkflowExecuteThreadPool workflowExecuteThreadPool;
+    private final ProcessInstanceExecCacheManager cacheManager;
+    private final String localAddress;
 
     public FailoverService(RegistryClient registryClient,
                            MasterConfig masterConfig,
                            ProcessService processService,
-                           WorkflowExecuteThreadPool workflowExecuteThreadPool) {
+                           WorkflowExecuteThreadPool workflowExecuteThreadPool,
+                           ProcessInstanceExecCacheManager cacheManager) {
         this.registryClient = checkNotNull(registryClient);
         this.masterConfig = checkNotNull(masterConfig);
         this.processService = checkNotNull(processService);
         this.workflowExecuteThreadPool = checkNotNull(workflowExecuteThreadPool);
+        this.cacheManager = checkNotNull(cacheManager);

Review Comment:
   nit: `@NonNull`



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