----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30306/#review69800 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java <https://reviews.apache.org/r/30306/#comment114597> Manual database edits are not something that we should support because they mess with JPA's caching. This work seems dangerous to do on a heartbeat as it involves a lot of work and queries. I'd rather you use an event to do this work asynchronously. ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java <https://reviews.apache.org/r/30306/#comment114599> Naming; this is confusing and hard to read. Instead, maybe make this compInfo.isVersionAdvertised() ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java <https://reviews.apache.org/r/30306/#comment114600> Does this code need a lock? You're querying and then adding if absent, but couldn't 2 heartbeats come in while you're doing this work? Locking would probably be good, especially since I'm recommending that you do this work in an async event, not from the heartbeat handler directly. ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java <https://reviews.apache.org/r/30306/#comment114598> Shouldn't the upgrade logic take care of this? - Jonathan Hurley On Jan. 26, 2015, 11:10 p.m., Alejandro Fernandez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30306/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 11:10 p.m.) > > > Review request for Ambari, Jonathan Hurley, Nate Cole, and Yurii Shylov. > > > Bugs: AMBARI-9340 > https://issues.apache.org/jira/browse/AMBARI-9340 > > > Repository: ambari > > > Description > ------- > > New cluster install through the Install Wizard correctly transitioned all of > the host_versions into CURRENT, but did not propagate the changes to the > cluster_version, which stayed stuck on UPGRADING, because one host only had > AMS, so it could not create a host_version record. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java > 092f9d54db36686412d7ccaa3318330688cf67cd > ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java > b344a052549bb393028a3d60ea9fdb0c2f70fde0 > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 220a5af9923444703eab4cff1875f19aeee08742 > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java > 372325df8b1e1191b87441304dc9a994f8b56f72 > > Diff: https://reviews.apache.org/r/30306/diff/ > > > Testing > ------- > > Installed a 3-node cluster with only HDFS, ZK, and AMS. One of the hosts only > had the AMS component. When the install finishes, the other 2 hosts correctly > had host_version records in CURRENT, and the cluster_version was in CURRENT. > After the install finishes, added a 4th node with all 3 components, and it > correctly created a host_version record in the CURRENT state. > > This means that the UI needs to be robust enough to handle the case where > some hosts don't have host_version records. > > Unit tests are in progress. > > > Thanks, > > Alejandro Fernandez > >
