----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62688/#review186766 -----------------------------------------------------------
Ship it! Ship It! - Jonathan Hurley On Sept. 29, 2017, 10:30 a.m., Attila Doroszlai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62688/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2017, 10:30 a.m.) > > > Review request for Ambari, Attila Magyar, Balázs Bence Sári, Jonathan Hurley, > and Sandor Magyari. > > > Bugs: AMBARI-22092 > https://issues.apache.org/jira/browse/AMBARI-22092 > > > Repository: ambari > > > Description > ------- > > Improved logging in `AsyncCallableService`: > * Add task name to all log messages, since it is a generic service (although > currently only used for a single task). > * Introduced new "marker" exception to indicate that task should be retried > silently, without logging the exception. > * `TimeoutException` is not an error, no need to log with stack trace, > timeout is logged anyway. > * For other exceptions peel off the `ExecutionException` that the executor > framework wraps the actual exception in to reduce verboseness. > > Improved host group-related messages: > * Warn if a host group is completely missing. > * Log completion of each host group only once. > * Log host groups still waiting for more hosts only once for each host count > (eg. log when waiting for 3 hosts, then only when waiting for 2 more hosts). > > Minor bug fixes: > * `AsyncCallableService.timeout` is the total time allowed for repeated task > execution. Hence, timeout for subsequent attempts (of `future.get()`) needs > to be reduced by the time passed since the task's initial attempt. > * `verify()` is a no-op (expects mocks as arg), `verifyAll()` was intended > to be called. > > Moved timeout config logic to `ConfigureClusterTask`. > Misc code cleanup. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/topology/AsyncCallableService.java > 9a68ea7cd9ee62250fde33ceb0cf2b65b57ae883 > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > 9769fae4b851d8bdf908c2917d7af57ff0ea061e > > ambari-server/src/main/java/org/apache/ambari/server/topology/tasks/ConfigureClusterTask.java > 60eaa591b9ac3b3ffa417d5c5d84144d4e68d9d1 > > ambari-server/src/test/java/org/apache/ambari/server/topology/AsyncCallableServiceTest.java > 4d962949400485f4d846d87ad74059e0c7eeb9df > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterDeployWithStartOnlyTest.java > 0daa20fc5e8d1a55818840407aff9ebd1703cf5c > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartOnComponentLevelTest.java > bbf4fdbf01b6498c897093c0efd7a0957e041975 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java > 059a8be735e193fe523a32df945a7566dfc22dc7 > > ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigureClusterTaskTest.java > b2dac8f01a817a1f9b0ee6f02a976cdd3304fd94 > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > ac643d7c661a1bcb8a0400bff94aeb257aec4101 > > > Diff: https://reviews.apache.org/r/62688/diff/2/ > > > Testing > ------- > > * Manually on cluster with Ambari 2.6: successful completion, successful on > retries and timeout. > * Unit tests (modified ones are OK; result of the rest is pending) > > > Thanks, > > Attila Doroszlai > >
