[ambari] 01/02: AMBARI-22805. Blueprints do not handle some failures properly - improve error message in case of timeout

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