[GitHub] [incubator-doris] pengxiangyu commented on a change in pull request #6705: [Bugs] Fix the bugs list of sync job

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

pengxiangyu commented on a change in pull request #6705:
URL: https://github.com/apache/incubator-doris/pull/6705#discussion_r714506283



##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJob.java
##########
@@ -152,6 +186,65 @@ public synchronized void updateState(JobState newState, boolean isReplay) {
         }
     }
 
+    private void checkStateTransform(JobState newState) throws UserException {
+        CHECK: {
+            switch (jobState) {
+                case RUNNING:
+                    if (newState == JobState.RUNNING) {
+                        break CHECK;

Review comment:
       这里直接throw比较好，这样不需要外面的CHECK，结尾处也不需要再return了。逻辑比较清晰

##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJobManager.java
##########
@@ -281,9 +296,6 @@ public void readField(DataInput in) throws IOException {
         int size = in.readInt();
         for (int i = 0; i < size; i++) {
             SyncJob syncJob = SyncJob.read(in);
-            if (!syncJob.isCompleted()) {
-                syncJob.updateState(JobState.PENDING, true);
-            }
             unprotectedAddSyncJob(syncJob);

Review comment:
       这里不用加锁么？调用unprotectedAddSyncJob的时候没有检查同名jobName是否已存在，应该要做下检查吧，否则同名任务会被直接覆盖。

##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncJob.java
##########
@@ -152,6 +186,65 @@ public synchronized void updateState(JobState newState, boolean isReplay) {
         }
     }
 
+    private void checkStateTransform(JobState newState) throws UserException {
+        CHECK: {
+            switch (jobState) {
+                case RUNNING:
+                    if (newState == JobState.RUNNING) {
+                        break CHECK;
+                    }
+                    break;
+                case PAUSED:
+                    if (newState == JobState.PAUSED || newState == JobState.RUNNING) {
+                        break CHECK;

Review comment:
       同上，这一段整体改造一下吧。

##########
File path: fe/fe-core/src/main/java/org/apache/doris/load/sync/SyncLifeCycle.java
##########
@@ -55,10 +55,6 @@ public void run() {
     }
 
     public void stop() {
-        if (!isStart()) {
-            throw new RuntimeException(this.getClass().getName() + " isn't start , please check");

Review comment:
       这块外面没有catch这个异常的地方，没什么意义。如果isStart()为false也存在重复stop的情况，建议直接按stop成功来处理。




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org