You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ad...@apache.org on 2018/01/24 15:37:28 UTC
[ambari] branch branch-2.6 updated (3730c03 -> 51add12)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a change to branch branch-2.6
in repository https://gitbox.apache.org/repos/asf/ambari.git.
from 3730c03 AMBARI-22696 Whitelist execute latency from Storm Ambari metrics
new c8921b6 AMBARI-22805. Blueprints do not handle some failures properly - improve error message in case of timeout
new 51add12 AMBARI-22805. Blueprints do not handle some failures properly - fix timeout edge case
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../server/topology/AsyncCallableService.java | 16 +++++++++---
.../topology/tasks/ConfigureClusterTask.java | 5 ++--
.../server/topology/AsyncCallableServiceTest.java | 30 ++++++++++++++++++++++
3 files changed, 45 insertions(+), 6 deletions(-)
--
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.
[ambari] 02/02: AMBARI-22805. Blueprints do not handle some
failures properly - fix timeout edge case
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch branch-2.6
in repository https://gitbox.apache.org/repos/asf/ambari.git
commit 51add1288529c2b28badf5737f50c40deb07e3a6
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Tue Jan 23 23:00:11 2018 +0100
AMBARI-22805. Blueprints do not handle some failures properly - fix timeout edge case
---
.../org/apache/ambari/server/topology/AsyncCallableService.java | 4 ++--
.../apache/ambari/server/topology/AsyncCallableServiceTest.java | 8 ++++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
index 1d8b322..8b24c65 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
@@ -100,10 +100,10 @@ public class AsyncCallableService<T> implements Callable<T> {
LOG.info(String.format("Task %s exception during execution", taskName), cause);
}
lastError = cause;
- timeLeft = timeout - (System.currentTimeMillis() - startTime);
+ timeLeft = timeout - (System.currentTimeMillis() - startTime) - retryDelay;
}
- if (timeLeft < retryDelay) {
+ if (timeLeft <= 0) {
attemptToCancel(future);
LOG.warn("Task {} timeout exceeded, no more retries", taskName);
onError.apply(lastError);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
index bc8ef2b..a4586b8 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
@@ -18,8 +18,9 @@
package org.apache.ambari.server.topology;
-import static org.easymock.EasyMock.anyLong;
import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.captureLong;
+import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import java.util.concurrent.Callable;
@@ -29,6 +30,7 @@ import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import org.easymock.Capture;
import org.easymock.EasyMockRule;
import org.easymock.EasyMockSupport;
import org.easymock.Mock;
@@ -90,7 +92,8 @@ public class AsyncCallableServiceTest extends EasyMockSupport {
Exception timeoutException = new TimeoutException("Timeout during second attempt");
expect(futureMock.get(TIMEOUT, TimeUnit.MILLISECONDS)).andThrow(computationException);
expect(executorServiceMock.schedule(taskMock, RETRY_DELAY, TimeUnit.MILLISECONDS)).andReturn(futureMock);
- expect(futureMock.get(anyLong(), anyObject(TimeUnit.class))).andThrow(timeoutException);
+ Capture<Long> timeoutCapture = Capture.newInstance();
+ expect(futureMock.get(captureLong(timeoutCapture), eq(TimeUnit.MILLISECONDS))).andThrow(timeoutException);
expect(futureMock.isDone()).andReturn(Boolean.FALSE);
expect(futureMock.cancel(true)).andReturn(Boolean.TRUE);
expect(executorServiceMock.submit(taskMock)).andReturn(futureMock);
@@ -104,6 +107,7 @@ public class AsyncCallableServiceTest extends EasyMockSupport {
// THEN
verifyAll();
+ Assert.assertTrue(timeoutCapture.getValue() <= TIMEOUT - RETRY_DELAY);
Assert.assertNull("No result expected in case of timeout", serviceResult);
}
--
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.
[ambari] 01/02: AMBARI-22805. Blueprints do not handle some
failures properly - improve error message in case of timeout
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch branch-2.6
in repository https://gitbox.apache.org/repos/asf/ambari.git
commit c8921b6046aadc712ac4b389e825bc482129d40d
Author: Doroszlai, Attila <ad...@apache.org>
AuthorDate: Tue Jan 23 19:16:54 2018 +0100
AMBARI-22805. Blueprints do not handle some failures properly - improve error message in case of timeout
---
.../server/topology/AsyncCallableService.java | 12 ++++++++--
.../topology/tasks/ConfigureClusterTask.java | 5 +++--
.../server/topology/AsyncCallableServiceTest.java | 26 ++++++++++++++++++++++
3 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
index 3abb52c..1d8b322 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java
@@ -81,8 +81,8 @@ public class AsyncCallableService<T> implements Callable<T> {
Future<T> future = executorService.submit(task);
LOG.info("Task {} execution started at {}", taskName, startTime);
+ Throwable lastError = null;
while (true) {
- Throwable lastError;
try {
LOG.debug("Task {} waiting for result at most {} ms", taskName, timeLeft);
T taskResult = future.get(timeLeft, TimeUnit.MILLISECONDS);
@@ -90,7 +90,9 @@ public class AsyncCallableService<T> implements Callable<T> {
return taskResult;
} catch (TimeoutException e) {
LOG.debug("Task {} timeout", taskName);
- lastError = e;
+ if (lastError == null) {
+ lastError = e;
+ }
timeLeft = 0;
} catch (ExecutionException e) {
Throwable cause = Throwables.getRootCause(e);
@@ -124,6 +126,12 @@ public class AsyncCallableService<T> implements Callable<T> {
public static class RetryTaskSilently extends RuntimeException {
// marker, throw if the task needs to be retried
+ public RetryTaskSilently() {
+ super();
+ }
+ public RetryTaskSilently(String message) {
+ super(message);
+ }
}
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/tasks/ConfigureClusterTask.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/tasks/ConfigureClusterTask.java
index 0f13ec2..ed1c451 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/tasks/ConfigureClusterTask.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/tasks/ConfigureClusterTask.java
@@ -71,8 +71,9 @@ public class ConfigureClusterTask implements Callable<Boolean> {
Collection<String> requiredHostGroups = getTopologyRequiredHostGroups();
if (!areHostGroupsResolved(requiredHostGroups)) {
- LOG.info("Some host groups require more hosts, cluster configuration cannot begin");
- throw new AsyncCallableService.RetryTaskSilently();
+ String msg = "Some host groups require more hosts, cluster configuration cannot begin";
+ LOG.info(msg);
+ throw new AsyncCallableService.RetryTaskSilently(msg);
}
LOG.info("All required host groups are complete, cluster configuration can now begin");
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
index 3930e2e..bc8ef2b 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java
@@ -18,10 +18,12 @@
package org.apache.ambari.server.topology;
+import static org.easymock.EasyMock.anyLong;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.expect;
import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
@@ -82,6 +84,30 @@ public class AsyncCallableServiceTest extends EasyMockSupport {
}
@Test
+ public void lastErrorIsReturnedIfSubsequentAttemptTimesOut() throws Exception {
+ // GIVEN
+ Exception computationException = new ExecutionException(new ArithmeticException("Computation error during first attempt"));
+ Exception timeoutException = new TimeoutException("Timeout during second attempt");
+ expect(futureMock.get(TIMEOUT, TimeUnit.MILLISECONDS)).andThrow(computationException);
+ expect(executorServiceMock.schedule(taskMock, RETRY_DELAY, TimeUnit.MILLISECONDS)).andReturn(futureMock);
+ expect(futureMock.get(anyLong(), anyObject(TimeUnit.class))).andThrow(timeoutException);
+ expect(futureMock.isDone()).andReturn(Boolean.FALSE);
+ expect(futureMock.cancel(true)).andReturn(Boolean.TRUE);
+ expect(executorServiceMock.submit(taskMock)).andReturn(futureMock);
+ expect(onErrorMock.apply(computationException.getCause())).andReturn(null);
+ replayAll();
+
+ asyncCallableService = new AsyncCallableService<>(taskMock, TIMEOUT, RETRY_DELAY, "test", executorServiceMock, onErrorMock);
+
+ // WHEN
+ Boolean serviceResult = asyncCallableService.call();
+
+ // THEN
+ verifyAll();
+ Assert.assertNull("No result expected in case of timeout", serviceResult);
+ }
+
+ @Test
public void testCallableServiceShouldCancelTaskWhenTaskHangsAndTimeoutExceeded() throws Exception {
// GIVEN
//the task call hangs, it doesn't return within a reasonable period of time
--
To stop receiving notification emails like this one, please contact
adoroszlai@apache.org.