You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kylin.apache.org by GitBox <gi...@apache.org> on 2018/08/28 03:08:38 UTC

[GitHub] shaofengshi closed pull request #208: KYLIN-3516 Fix job status not updated after job discarded

shaofengshi closed pull request #208: KYLIN-3516 Fix job status not updated after job discarded
URL: https://github.com/apache/kylin/pull/208
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
index 54076493bc..efe9e0ebda 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
@@ -160,7 +160,8 @@ public JobInstance cancel(@PathVariable String jobId) {
 
         try {
             final JobInstance jobInstance = jobService.getJobInstance(jobId);
-            return jobService.cancelJob(jobInstance);
+            jobService.cancelJob(jobInstance);
+            return jobService.getJobInstance(jobId);
         } catch (Exception e) {
             logger.error(e.getLocalizedMessage(), e);
             throw new InternalErrorException(e);
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/JobService.java b/server-base/src/main/java/org/apache/kylin/rest/service/JobService.java
index 2603267dd2..8f8658c71b 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/JobService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/JobService.java
@@ -559,12 +559,11 @@ public void rollbackJob(JobInstance job, String stepId) {
         getExecutableManager().rollbackJob(job.getId(), stepId);
     }
 
-    public JobInstance cancelJob(JobInstance job) throws IOException {
+    public void cancelJob(JobInstance job) throws IOException {
         aclEvaluate.checkProjectOperationPermission(job);
         if (null == job.getRelatedCube() || null == getCubeManager().getCube(job.getRelatedCube())
                 || null == job.getRelatedSegment()) {
             getExecutableManager().discardJob(job.getId());
-            return job;
         }
 
         logger.info("Cancel job [" + job.getId() + "] trigger by "
@@ -573,19 +572,17 @@ public JobInstance cancelJob(JobInstance job) throws IOException {
             throw new IllegalStateException(
                     "The job " + job.getId() + " has already been finished and cannot be discarded.");
         }
-        if (job.getStatus() == JobStatusEnum.DISCARDED) {
-            return job;
-        }
 
-        AbstractExecutable executable = getExecutableManager().getJob(job.getId());
-        if (executable instanceof CubingJob) {
-            cancelCubingJobInner((CubingJob) executable);
-        } else if (executable instanceof CheckpointExecutable) {
-            cancelCheckpointJobInner((CheckpointExecutable) executable);
-        } else {
-            getExecutableManager().discardJob(executable.getId());
+        if (job.getStatus() != JobStatusEnum.DISCARDED) {
+            AbstractExecutable executable = getExecutableManager().getJob(job.getId());
+            if (executable instanceof CubingJob) {
+                cancelCubingJobInner((CubingJob) executable);
+            } else if (executable instanceof CheckpointExecutable) {
+                cancelCheckpointJobInner((CheckpointExecutable) executable);
+            } else {
+                getExecutableManager().discardJob(executable.getId());
+            }
         }
-        return job;
     }
 
     private void cancelCubingJobInner(CubingJob cubingJob) throws IOException {
diff --git a/server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java b/server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java
index 436474396e..439381efa3 100644
--- a/server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java
+++ b/server/src/test/java/org/apache/kylin/rest/controller/JobControllerTest.java
@@ -18,12 +18,16 @@
 
 package org.apache.kylin.rest.controller;
 
+import java.io.IOException;
+import java.util.Date;
+
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.cube.CubeDescManager;
 import org.apache.kylin.cube.CubeInstance;
 import org.apache.kylin.cube.CubeManager;
 import org.apache.kylin.cube.model.CubeDesc;
 import org.apache.kylin.job.JobInstance;
+import org.apache.kylin.job.constant.JobStatusEnum;
 import org.apache.kylin.job.dao.ExecutableDao;
 import org.apache.kylin.job.exception.PersistentException;
 import org.apache.kylin.rest.request.JobBuildRequest;
@@ -38,11 +42,6 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 
-import java.io.IOException;
-import java.util.Date;
-
-import static org.junit.Assert.assertNotNull;
-
 /**
  * @author xduo
  */
@@ -91,7 +90,7 @@ public void tearDown() throws Exception {
     public void testBasics() throws IOException, PersistentException {
         CubeDesc cubeDesc = cubeDescManager.getCubeDesc("test_kylin_cube_with_slr_left_join_desc");
         CubeInstance cube = cubeManager.createCube(CUBE_NAME, "DEFAULT", cubeDesc, "test");
-        assertNotNull(cube);
+        Assert.assertNotNull(cube);
 
         JobListRequest jobRequest = new JobListRequest();
         jobRequest.setTimeFilter(4);
@@ -100,11 +99,9 @@ public void testBasics() throws IOException, PersistentException {
         jobRequest.setJobSearchMode("ALL");
         Assert.assertNotNull(jobSchedulerController.list(jobRequest));
 
-
         jobRequest.setJobSearchMode("");
         Assert.assertNotNull(jobSchedulerController.list(jobRequest));
 
-
         jobRequest.setJobSearchMode("wrong-input");
         Assert.assertNotNull(jobSchedulerController.list(jobRequest));
 
@@ -115,12 +112,14 @@ public void testBasics() throws IOException, PersistentException {
         JobInstance job = cubeController.rebuild(CUBE_NAME, jobBuildRequest);
 
         Assert.assertNotNull(jobSchedulerController.get(job.getId()));
+
+        job = jobSchedulerController.cancel(job.getId());
+        Assert.assertEquals(JobStatusEnum.DISCARDED, job.getStatus());
+
         executableDAO.deleteJob(job.getId());
         if (cubeManager.getCube(CUBE_NAME) != null) {
             cubeManager.dropCube(CUBE_NAME, false);
         }
-
-        // jobSchedulerController.cancel(job.getId());
     }
 
     @Test(expected = RuntimeException.class)


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services