You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@ambari.apache.org by Attila Doroszlai <ad...@hortonworks.com> on 2017/09/29 13:06:02 UTC

Review Request 62688: AMBARI-22092. Blueprint cluster creation constantly throwing exceptions

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62688/
-----------------------------------------------------------

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

Diff: https://reviews.apache.org/r/62688/diff/1/

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

Re: Review Request 62688: AMBARI-22092. Blueprint cluster creation constantly throwing exceptions

Posted by Sandor Magyari <sm...@hortonworks.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62688/#review186672
-----------------------------------------------------------


Ship it!




Ship It!

- Sandor Magyari


On Sept. 29, 2017, 1:06 p.m., Attila Doroszlai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62688/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2017, 1:06 p.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/ConfigureClusterTaskTest.java b2dac8f01a817a1f9b0ee6f02a976cdd3304fd94 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java ac643d7c661a1bcb8a0400bff94aeb257aec4101 
> 
> 
> Diff: https://reviews.apache.org/r/62688/diff/1/
> 
> 
> 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
> 
>

Re: Review Request 62688: AMBARI-22092. Blueprint cluster creation constantly throwing exceptions

Posted by Jonathan Hurley <jh...@hortonworks.com>.

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

Re: Review Request 62688: AMBARI-22092. Blueprint cluster creation constantly throwing exceptions

Posted by Attila Doroszlai <ad...@hortonworks.com>.

(Updated Sept. 29, 2017, 4:30 p.m.)

Review request for Ambari, Attila Magyar, Balázs Bence Sári, Jonathan Hurley, and Sandor Magyari.

Changes
-------

fixed 3 unit tests

Bugs: AMBARI-22092
https://issues.apache.org/jira/browse/AMBARI-22092

Repository: ambari

Description
-------

Moved timeout config logic to `ConfigureClusterTask`.
Misc code cleanup.

Diffs (updated)
-----

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/

Changes: https://reviews.apache.org/r/62688/diff/1-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