----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30912/#review72132 -----------------------------------------------------------
Ship it! Minor comments. ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java <https://reviews.apache.org/r/30912/#comment118145> Should this be a class level constant? private static final String SECONDARY_NAMENODE = "SECONDARY_NAMENODE"; ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java <https://reviews.apache.org/r/30912/#comment118152> If you suspect that the first check may be unreliable at this point, why leave it in here? Wouldn't the second check be sufficient? ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java <https://reviews.apache.org/r/30912/#comment118154> So Service.getServiceComponent() throws an exception to indicate that it doesn't have the requested service? Strange. I wonder why it just doesn't return null. I would say that if that is the behavior then you don't need the null check also, but we don't know if that is the behavior since that method in the public Service interface has no documentation. ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java <https://reviews.apache.org/r/30912/#comment118146> +1 for adding a comment here. Drop the unnecessary semicolon. - Tom Beerbower On Feb. 12, 2015, 2:37 a.m., Alejandro Fernandez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30912/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2015, 2:37 a.m.) > > > Review request for Ambari, Dmitro Lisnichenko, Jonathan Hurley, Nate Cole, > Robert Levas, and Tom Beerbower. > > > Bugs: AMBARI-9559 > https://issues.apache.org/jira/browse/AMBARI-9559 > > > Repository: ambari > > > Description > ------- > > I reopened AMBARI-9559 for this additional patch. > > While performing a RU, it failed in Finalize because the SECONDARY_NAMENODE > component was still on an old version, since it was not deleted by the > Namenode HA wizard. > This was a 3-node cluster with HDFS, MR, YARN, TEZ, Hive, ZK, Pig, AMS using > HDP 2.2.0.0 GA. > > I was not able to reproduce this, even after installing the exact same > services on the same hosts, and using the same ambari-server version. It > could be the case that some thread is holding a reference to the object that > is being deleted, so it persists. So I added another check to > PreUpgradeCheckResourceProvider that ensures that SECONDARY_NAMENODE is not > present on any hosts. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/checks/SecondaryNamenodeDeletedCheck.java > ad967f8 > > Diff: https://reviews.apache.org/r/30912/diff/ > > > Testing > ------- > > Deployed a 3-node cluster with HDFS, ZK, and added Namenode HA. I was able to > reproduce SECONDARY_NAMENODE not being deleted. > I then registered a repo and distributed it to the hosts, and when I tried to > perform a RU, the precheck correctly reported an error. > > Unit tests are in progress. > > > Thanks, > > Alejandro Fernandez > >
