This is an automated email from the ASF dual-hosted git repository.
billyliu pushed a commit to branch 2.3.x
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 57462faa7c96ff9bfbe54048e42c486f5512abe8
Author: Jiatao Tao <245915...@qq.com>
AuthorDate: Fri Feb 23 16:37:16 2018 +0800
KYLIN-3263, bugfix with AbstractExecutable's retry.
---
.../kylin/job/execution/AbstractExecutable.java | 32 +++++---------
.../apache/kylin/job/RetryableTestExecutable.java | 50 ----------------------
.../job/impl/threadpool/DefaultSchedulerTest.java | 49 +++++++++++----------
3 files changed, 37 insertions(+), 94 deletions(-)
diff --git
a/core-job/src/main/java/org/apache/kylin/job/execution/AbstractExecutable.java
b/core-job/src/main/java/org/apache/kylin/job/execution/AbstractExecutable.java
index 91283f0..dbe11c2 100644
---
a/core-job/src/main/java/org/apache/kylin/job/execution/AbstractExecutable.java
+++
b/core-job/src/main/java/org/apache/kylin/job/execution/AbstractExecutable.java
@@ -165,7 +165,7 @@ public abstract class AbstractExecutable implements
Executable, Idempotent {
exception = e;
}
retry++;
- } while (needRetry(result, exception));
+ } while (needRetry(this.retry, exception)); //exception in
ExecuteResult should handle by user itself.
if (exception != null) {
onExecuteError(exception, executableContext);
@@ -221,13 +221,6 @@ public abstract class AbstractExecutable implements
Executable, Idempotent {
return false;
}
- private boolean isRetryableExecutionResult(ExecuteResult result) {
- if (result != null && result.getThrowable() != null &&
isRetrableException(result.getThrowable())) {
- return true;
- }
- return false;
- }
-
protected abstract ExecuteResult doWork(ExecutableContext context) throws
ExecuteException;
@Override
@@ -468,25 +461,20 @@ public abstract class AbstractExecutable implements
Executable, Idempotent {
return status == ExecutableState.STOPPED;
}
- protected boolean isRetrableException(Throwable t) {
- return
ArrayUtils.contains(KylinConfig.getInstanceFromEnv().getJobRetryExceptions(),
t.getClass().getName());
- }
-
// Retry will happen in below cases:
// 1) if property "kylin.job.retry-exception-classes" is not set or is
null, all jobs with exceptions will retry according to the retry times.
// 2) if property "kylin.job.retry-exception-classes" is set and is not
null, only jobs with the specified exceptions will retry according to the retry
times.
- protected boolean needRetry(ExecuteResult result, Throwable e) {
- if (this.retry > KylinConfig.getInstanceFromEnv().getJobRetry()) {
+ public static boolean needRetry(int retry, Throwable t) {
+ if (retry > KylinConfig.getInstanceFromEnv().getJobRetry() || t ==
null) {
return false;
+ } else {
+ return isRetryableException(t.getClass().getName());
}
- String[] retryableEx =
KylinConfig.getInstanceFromEnv().getJobRetryExceptions();
- if (retryableEx == null || retryableEx.length == 0) {
- return true;
- }
- if ((result != null && isRetryableExecutionResult(result)) || e !=
null && isRetrableException(e)) {
- return true;
- }
- return false;
+ }
+
+ private static boolean isRetryableException(String exceptionName) {
+ String[] jobRetryExceptions =
KylinConfig.getInstanceFromEnv().getJobRetryExceptions();
+ return ArrayUtils.isEmpty(jobRetryExceptions) ||
ArrayUtils.contains(jobRetryExceptions, exceptionName);
}
@Override
diff --git
a/core-job/src/test/java/org/apache/kylin/job/RetryableTestExecutable.java
b/core-job/src/test/java/org/apache/kylin/job/RetryableTestExecutable.java
deleted file mode 100644
index f656c44..0000000
--- a/core-job/src/test/java/org/apache/kylin/job/RetryableTestExecutable.java
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.kylin.job;
-
-import org.apache.kylin.common.KylinConfig;
-import org.apache.kylin.job.execution.ExecutableContext;
-import org.apache.kylin.job.execution.ExecuteResult;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- */
-public class RetryableTestExecutable extends BaseTestExecutable {
- private static final Logger logger =
LoggerFactory.getLogger(RetryableTestExecutable.class);
-
- public RetryableTestExecutable() {
- super();
- }
-
- @Override
- protected ExecuteResult doWork(ExecutableContext context) {
- logger.debug("run retryable exception test. ");
- String[] exceptions =
KylinConfig.getInstanceFromEnv().getJobRetryExceptions();
- Throwable ex = null;
- if (exceptions != null && exceptions.length > 0) {
- try {
- ex = (Throwable) Class.forName(exceptions[0]).newInstance();
- } catch (Exception e) {
- e.printStackTrace();
- }
- }
- return new ExecuteResult(ExecuteResult.State.ERROR, null, ex);
- }
-}
diff --git
a/core-job/src/test/java/org/apache/kylin/job/impl/threadpool/DefaultSchedulerTest.java
b/core-job/src/test/java/org/apache/kylin/job/impl/threadpool/DefaultSchedulerTest.java
index c7c69cd..3b24fe6 100644
---
a/core-job/src/test/java/org/apache/kylin/job/impl/threadpool/DefaultSchedulerTest.java
+++
b/core-job/src/test/java/org/apache/kylin/job/impl/threadpool/DefaultSchedulerTest.java
@@ -18,25 +18,16 @@
package org.apache.kylin.job.impl.threadpool;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
-
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.job.BaseTestExecutable;
import org.apache.kylin.job.ErrorTestExecutable;
import org.apache.kylin.job.FailedTestExecutable;
import org.apache.kylin.job.FiveSecondSucceedTestExecutable;
import org.apache.kylin.job.NoErrorStatusExecutable;
-import org.apache.kylin.job.RetryableTestExecutable;
import org.apache.kylin.job.RunningTestExecutable;
import org.apache.kylin.job.SelfStopExecutable;
import org.apache.kylin.job.SucceedTestExecutable;
+import org.apache.kylin.job.execution.AbstractExecutable;
import org.apache.kylin.job.execution.DefaultChainedExecutable;
import org.apache.kylin.job.execution.ExecutableManager;
import org.apache.kylin.job.execution.ExecutableState;
@@ -48,11 +39,28 @@ import org.junit.rules.ExpectedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.FileNotFoundException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
/**
*/
public class DefaultSchedulerTest extends BaseSchedulerTest {
private static final Logger logger =
LoggerFactory.getLogger(DefaultSchedulerTest.class);
+ @Override
+ public void after() throws Exception {
+ super.after();
+ System.clearProperty("kylin.job.retry");
+ System.clearProperty("kylin.job.retry-exception-classes");
+ }
+
@Rule
public ExpectedException thrown = ExpectedException.none();
@@ -227,19 +235,16 @@ public class DefaultSchedulerTest extends
BaseSchedulerTest {
Assert.assertEquals(ExecutableState.SUCCEED,
execMgr.getOutput(job.getId()).getState());
Assert.assertEquals(ExecutableState.SUCCEED,
execMgr.getOutput(task1.getId()).getState());
}
-
+
+ @Test
public void testRetryableException() throws Exception {
- System.setProperty("kylin.job.retry-exception-classes",
"java.io.FileNotFoundException");
System.setProperty("kylin.job.retry", "3");
- DefaultChainedExecutable job = new DefaultChainedExecutable();
- BaseTestExecutable task1 = new SucceedTestExecutable();
- BaseTestExecutable task2 = new RetryableTestExecutable();
- job.addTask(task1);
- job.addTask(task2);
- execMgr.addJob(job);
- waitForJobFinish(job.getId(), 10000);
- Assert.assertEquals(ExecutableState.SUCCEED,
execMgr.getOutput(task1.getId()).getState());
- Assert.assertEquals(ExecutableState.ERROR,
execMgr.getOutput(task2.getId()).getState());
- Assert.assertEquals(ExecutableState.ERROR,
execMgr.getOutput(job.getId()).getState());
+ Assert.assertTrue(AbstractExecutable.needRetry(1, new Exception("")));
+ Assert.assertFalse(AbstractExecutable.needRetry(1, null));
+ Assert.assertFalse(AbstractExecutable.needRetry(4, new Exception("")));
+
+ System.setProperty("kylin.job.retry-exception-classes",
"java.io.FileNotFoundException");
+ Assert.assertTrue(AbstractExecutable.needRetry(1, new
FileNotFoundException()));
+ Assert.assertFalse(AbstractExecutable.needRetry(1, new Exception("")));
}
}
--
To stop receiving notification emails like this one, please contact
billy...@apache.org.
