You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2016/03/08 15:28:50 UTC
Re: Review Request 44492: [DRAFT] AMBARI-15330. Bubble up errors
during RU/EU
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44492/#review122518
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java (lines 51 - 57)
<https://reviews.apache.org/r/44492/#comment184563>
GET doesn't need a body
ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java (lines 60 - 69)
<https://reviews.apache.org/r/44492/#comment184564>
GET doesn't need a body
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java (line 25)
<https://reviews.apache.org/r/44492/#comment184568>
This UpgradeSummary class kind of assumes a failure. Perhaps it shouldn't? Or maybe we need a better layer of abstraction here. The summary of an upgrade could be a success, right? In which case it wouldn't have things like a taskId or stageId.
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java (lines 79 - 83)
<https://reviews.apache.org/r/44492/#comment184567>
Theses should already be part of the HostRoleStatus enum, no? If not, we should expose them there to be re-used by other areas.
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java (lines 706 - 709)
<https://reviews.apache.org/r/44492/#comment184565>
Make this a NamedQuery
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java (lines 722 - 725)
<https://reviews.apache.org/r/44492/#comment184566>
Make this a NamedQuery
- Jonathan Hurley
On March 7, 2016, 11:51 p.m., Alejandro Fernandez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44492/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 11:51 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan Hurley, Nate Cole, and Sid Wagle.
>
>
> Bugs: AMBARI-15330
> https://issues.apache.org/jira/browse/AMBARI-15330
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During RU/EU, need a way to bubble up an error of the current item that failed. This is useful to quickly get a human-readable error that others UIs can quickly retrieve.
> It can print a human-readable error, plus stdout and stderr.
> This would become part of the upgrade endpoint. e.g,
> api/v1/clusters/$name/upgrade_summary/$request_id
>
> ```
> {
>
> cluster_name: "c1",
> request_id: 1,
> fail_reason: "Failed calling RESTART ZOOKEEPER/ZOOKEEPER_SERVER on host c6401.ambari.apache.org",
>
> // Notice that the rest are inherited from the failed task if it exists.
> attempt_cnt: 1,
> command: "CUSTOM_COMMAND",
> command_detail: "RESTART ZOOKEEPER/ZOOKEEPER_SERVER",
> custom_command_name: "RESTART",
> end_time: -1,
> error_log: "/var/lib/ambari-agent/data/errors-1234.txt",
> exit_code: 1,
> host_name: "c6401.ambari.apache.org",
> id: 1234,
> output_log: "/var/lib/ambari-agent/data/output-1234.txt",
> role: "ZOOKEEPER_SERVER",
> stage_id: 1,
> start_time: 123456789,
> status: "HOLDING_FAILED",
> stdout: "",
> stderr: ""
> }
> ```
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java 3526e23
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 7200b83
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java d1d3fe6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TaskResourceProvider.java 510d6fb
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java e79f300
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java b48ffa8
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 05d49c4
>
> Diff: https://reviews.apache.org/r/44492/diff/
>
>
> Testing
> -------
>
> Verified on RU and EU by introducing a failure.
>
> Still need to write unit tests.
> ======
>
>
> Thanks,
>
> Alejandro Fernandez
>
>
Re: Review Request 44492: AMBARI-15330. Bubble up errors during RU/EU
Posted by Alejandro Fernandez <af...@hortonworks.com>.
> On March 8, 2016, 2:28 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java, line 25
> > <https://reviews.apache.org/r/44492/diff/2/?file=1290960#file1290960line25>
> >
> > This UpgradeSummary class kind of assumes a failure. Perhaps it shouldn't? Or maybe we need a better layer of abstraction here. The summary of an upgrade could be a success, right? In which case it wouldn't have things like a taskId or stageId.
>
> Alejandro Fernandez wrote:
> I tried adding a failed_task subresource and I had a hard time getting all of the boilerplate code added.
>
> Jonathan Hurley wrote:
> But this isn't really a summary, then. It's a failure only. If you don't want to go the subresource route, then maybe change this to reflect that you're only exposing an error condition. On a completed upgrade, it there won't be a summary. Maybe call it UpgradeFailureSummary? I dunno; but we should try to model this data bit better so that it represents either a failure or is flexible enough to represent both a failure and a success.
I got it working by just using a failed_task subelement in the json instead of a TaskSubresource. Much simpler since I don't actually need to query on it. Thanks for the tip!
- Alejandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44492/#review122518
-----------------------------------------------------------
On March 10, 2016, 10:48 p.m., Alejandro Fernandez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44492/
> -----------------------------------------------------------
>
> (Updated March 10, 2016, 10:48 p.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan Hurley, Nate Cole, and Sid Wagle.
>
>
> Bugs: AMBARI-15330
> https://issues.apache.org/jira/browse/AMBARI-15330
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During RU/EU, need a way to bubble up an error of the current item that failed. This is useful to quickly get a human-readable error that others UIs can quickly retrieve.
> It can print a human-readable error, plus stdout and stderr.
> This would become part of the upgrade endpoint. e.g,
> api/v1/clusters/$name/upgrade_summary/$request_id
>
> ```
> {
>
> cluster_name: "c1",
> request_id: 1,
> fail_reason: "Failed calling RESTART ZOOKEEPER/ZOOKEEPER_SERVER on host c6401.ambari.apache.org",
>
> // Notice that the rest are inherited from the failed task if it exists.
> attempt_cnt: 1,
> command: "CUSTOM_COMMAND",
> command_detail: "RESTART ZOOKEEPER/ZOOKEEPER_SERVER",
> custom_command_name: "RESTART",
> end_time: -1,
> error_log: "/var/lib/ambari-agent/data/errors-1234.txt",
> exit_code: 1,
> host_name: "c6401.ambari.apache.org",
> id: 1234,
> output_log: "/var/lib/ambari-agent/data/output-1234.txt",
> role: "ZOOKEEPER_SERVER",
> stage_id: 1,
> start_time: 123456789,
> status: "HOLDING_FAILED",
> stdout: "",
> stderr: ""
> }
> ```
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java 52523c7
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java 3526e23
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 7200b83
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java d1d3fe6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TaskResourceProvider.java 510d6fb
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java e79f300
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java b48ffa8
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java 1674175
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 2ac4d25
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProviderTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44492/diff/
>
>
> Testing
> -------
>
> Verified on RU and EU by introducing a failure.
> Waiting for unit test results.
>
>
> Thanks,
>
> Alejandro Fernandez
>
>
Re: Review Request 44492: [DRAFT] AMBARI-15330. Bubble up errors
during RU/EU
Posted by Alejandro Fernandez <af...@hortonworks.com>.
> On March 8, 2016, 2:28 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java, line 25
> > <https://reviews.apache.org/r/44492/diff/2/?file=1290960#file1290960line25>
> >
> > This UpgradeSummary class kind of assumes a failure. Perhaps it shouldn't? Or maybe we need a better layer of abstraction here. The summary of an upgrade could be a success, right? In which case it wouldn't have things like a taskId or stageId.
I tried adding a failed_task subresource and I had a hard time getting all of the boilerplate code added.
> On March 8, 2016, 2:28 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java, lines 51-57
> > <https://reviews.apache.org/r/44492/diff/2/?file=1290957#file1290957line51>
> >
> > GET doesn't need a body
I tried without the GET and it didn't work.
> On March 8, 2016, 2:28 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java, lines 60-69
> > <https://reviews.apache.org/r/44492/diff/2/?file=1290957#file1290957line60>
> >
> > GET doesn't need a body
I tried without the GET and it didn't work.
- Alejandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44492/#review122518
-----------------------------------------------------------
On March 8, 2016, 4:51 a.m., Alejandro Fernandez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44492/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 4:51 a.m.)
>
>
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan Hurley, Nate Cole, and Sid Wagle.
>
>
> Bugs: AMBARI-15330
> https://issues.apache.org/jira/browse/AMBARI-15330
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During RU/EU, need a way to bubble up an error of the current item that failed. This is useful to quickly get a human-readable error that others UIs can quickly retrieve.
> It can print a human-readable error, plus stdout and stderr.
> This would become part of the upgrade endpoint. e.g,
> api/v1/clusters/$name/upgrade_summary/$request_id
>
> ```
> {
>
> cluster_name: "c1",
> request_id: 1,
> fail_reason: "Failed calling RESTART ZOOKEEPER/ZOOKEEPER_SERVER on host c6401.ambari.apache.org",
>
> // Notice that the rest are inherited from the failed task if it exists.
> attempt_cnt: 1,
> command: "CUSTOM_COMMAND",
> command_detail: "RESTART ZOOKEEPER/ZOOKEEPER_SERVER",
> custom_command_name: "RESTART",
> end_time: -1,
> error_log: "/var/lib/ambari-agent/data/errors-1234.txt",
> exit_code: 1,
> host_name: "c6401.ambari.apache.org",
> id: 1234,
> output_log: "/var/lib/ambari-agent/data/output-1234.txt",
> role: "ZOOKEEPER_SERVER",
> stage_id: 1,
> start_time: 123456789,
> status: "HOLDING_FAILED",
> stdout: "",
> stderr: ""
> }
> ```
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java 3526e23
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 7200b83
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java d1d3fe6
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TaskResourceProvider.java 510d6fb
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java e79f300
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java b48ffa8
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 05d49c4
>
> Diff: https://reviews.apache.org/r/44492/diff/
>
>
> Testing
> -------
>
> Verified on RU and EU by introducing a failure.
>
> Still need to write unit tests.
> ======
>
>
> Thanks,
>
> Alejandro Fernandez
>
>