You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Dmitro Lisnichenko <dl...@hortonworks.com> on 2016/04/21 13:14:44 UTC
Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/
-----------------------------------------------------------
Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
Bugs: AMBARI-16005
https://issues.apache.org/jira/browse/AMBARI-16005
Repository: ambari
Description
-------
Enhance Health Check for the Cluster before upgrading.
* check for alerts (perhaps don't start RU/EU if any blocker alerts).
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/46484/diff/
Testing
-------
mvn clean test
Thanks,
Dmitro Lisnichenko
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review129891
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (line 74)
<https://reviews.apache.org/r/46484/#comment193443>
CLUSTER type checks typically don't have hosts on getFailedOn() (at least, that's not the intent). SERVICE types get service names, HOST types get host names.
- Nate Cole
On April 21, 2016, 7:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review130145
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On April 21, 2016, 11:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review130469
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On April 25, 2016, 2:43 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 25, 2016, 2:43 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 4967834
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review130471
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On April 25, 2016, 10:43 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 25, 2016, 10:43 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 4967834
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/
-----------------------------------------------------------
(Updated April 25, 2016, 5:43 p.m.)
Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
Changes
-------
Fixed comments
Bugs: AMBARI-16005
https://issues.apache.org/jira/browse/AMBARI-16005
Repository: ambari
Description
-------
Enhance Health Check for the Cluster before upgrading.
* check for alerts (perhaps don't start RU/EU if any blocker alerts).
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 4967834
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/46484/diff/
Testing
-------
mvn clean test
Thanks,
Dmitro Lisnichenko
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
> On April 21, 2016, 8:57 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java, line 67
> > <https://reviews.apache.org/r/46484/diff/1/?file=1354793#file1354793line67>
> >
> > I thought we were going to filter for only a handful of alerts based on the name.
> > E.g., out of disk space, ambari performance, etc.
Finally we decided to warn about all alerts (as stated by Jonathan in last comment of jira)
- Dmitro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review129931
-----------------------------------------------------------
On April 21, 2016, 2:14 p.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 2:14 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review129931
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (line 67)
<https://reviews.apache.org/r/46484/#comment193484>
I thought we were going to filter for only a handful of alerts based on the name.
E.g., out of disk space, ambari performance, etc.
- Alejandro Fernandez
On April 21, 2016, 11:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 11:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review129890
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (lines 33 - 34)
<https://reviews.apache.org/r/46484/#comment193441>
I've usually seen imports ordered like so:
import static first
import java...
import org...
import com...
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (lines 71 - 82)
<https://reviews.apache.org/r/46484/#comment193442>
A check like this can use some formatted output. We can add any object to the getFailedDetail() that will get output as json. This give the UI the option to do something interesting with text.
See for example ConfigurationMergeCheck.
- Nate Cole
On April 21, 2016, 7:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review129888
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java (line 51)
<https://reviews.apache.org/r/46484/#comment193436>
Grammar: Cluster Health
I don't think we want to indicate here that it needs to be re-run; running it again won't really help unless action is taken first. Let's keep this as a general description.
ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java (line 54)
<https://reviews.apache.org/r/46484/#comment193437>
The following issues have been detected on this cluster and should be addressed before upgrading:
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (line 43)
<https://reviews.apache.org/r/46484/#comment193438>
Doc.
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (line 67)
<https://reviews.apache.org/r/46484/#comment193439>
This needs to be "current" alerts:
`AlertsDAO.findCurrentByCluster()`
This could potentially pull back 100,000 entries.
ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java (lines 72 - 75)
<https://reviews.apache.org/r/46484/#comment193440>
Let's do something better here; maybe add some formatting. Also - the text of the alert isn't necessary, just the definition label:
"<state>: <definition-label>: <host>"
The host may be null, so you'll need to account for that. Also, let's use a sorted set so that all CRITICALs and grouped and all WARNINGs are grouped.
- Jonathan Hurley
On April 21, 2016, 7:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>
Re: Review Request 46484: Enhance Health Check for the Cluster before
upgrading.
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46484/#review130081
-----------------------------------------------------------
One other thing is that you'll want to check against the maintenance mode of the current alert as well. If they put a host/component into maintenance mode, then we shouldn't warn on alerts which are affected.
- Jonathan Hurley
On April 21, 2016, 7:14 a.m., Dmitro Lisnichenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46484/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 7:14 a.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
>
>
> Bugs: AMBARI-16005
> https://issues.apache.org/jira/browse/AMBARI-16005
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Enhance Health Check for the Cluster before upgrading.
>
> * check for alerts (perhaps don't start RU/EU if any blocker alerts).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 3e957b1
> ambari-server/src/main/java/org/apache/ambari/server/checks/HealthCheck.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/checks/HealthCheckTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/46484/diff/
>
>
> Testing
> -------
>
> mvn clean test
>
>
> Thanks,
>
> Dmitro Lisnichenko
>
>