----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32414/#review77558 -----------------------------------------------------------
Ship it! Some minor nits and a few general questions just asking if the behavior is expected. ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java <https://reviews.apache.org/r/32414/#comment125671> Any reason this is a Boolean object? Seems like it could cause NPEs in when used by other areas of code. ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java <https://reviews.apache.org/r/32414/#comment125674> Same as above; any reason this can't be a primitive to avoid NPEs and null checks? ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java <https://reviews.apache.org/r/32414/#comment125675> Slightly confusing; the first time I read this, I thought these were the services that need a restart. But they are just the ones that are rack sensitive. Can we rename this Set to something like `rackSenstiveServices` ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/hdfs_namenode.py <https://reviews.apache.org/r/32414/#comment125676> Correct global imports. ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/hdfs_namenode.py <https://reviews.apache.org/r/32414/#comment125677> If I'm reading this right, it's going to generate a new topology script/mapping every time NN is started. Is that what we want even if the environment is not rack aware? - Jonathan Hurley On March 23, 2015, 4:35 p.m., Tom Beerbower wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32414/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 4:35 p.m.) > > > Review request for Ambari, Jonathan Hurley, John Speidel, and Nate Cole. > > > Bugs: AMBARI-6466 > https://issues.apache.org/jira/browse/AMBARI-6466 > > > Repository: ambari > > > Description > ------- > > 1. Generate topology script used by Hadoop to determine the rack location of > nodes. > 2. Update core-site.xml to point to script. > 3. Provide mechanism for marking services that need restart when host rack > info changes. > 4. React to host resource rack changes to ... > a. mark services needing restart. > b. update host/rack data for topology script > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/CustomServiceOrchestrator.py > 8c21503 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java > 982f10f > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java > 6d35a36 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java > 98390fd > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java > 3a359e5 > > ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java > ab1ede5 > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java > 939a496 > ambari-server/src/main/java/org/apache/ambari/server/utils/StageUtils.java > de84f35 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/configuration/core-site.xml > c0cd6b3 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml > 916d9b0 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/files/topology_script.py > PRE-CREATION > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/hdfs_namenode.py > 615dd54 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/params.py > d20e8e0 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/templates/topology_mappings.data.j2 > PRE-CREATION > > ambari-server/src/main/resources/common-services/YARN/2.1.0.2.0/metainfo.xml > 974165c > > ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java > 8f7c199 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java > 208218c > > ambari-server/src/test/java/org/apache/ambari/server/state/ServiceInfoTest.java > PRE-CREATION > ambari-server/src/test/python/stacks/2.0.6/HDFS/test_namenode.py 1919958 > ambari-server/src/test/python/stacks/2.0.6/configs/default.json d1701bd > > ambari-server/src/test/python/stacks/2.0.6/configs/ha_bootstrap_active_node.json > b3e61bc > > ambari-server/src/test/python/stacks/2.0.6/configs/ha_bootstrap_standby_node.json > 4bbb0f5 > ambari-server/src/test/python/stacks/2.0.6/configs/ha_default.json 652ca7b > ambari-server/src/test/python/stacks/2.0.6/configs/ha_secured.json 6c06f35 > ambari-server/src/test/python/stacks/2.0.6/configs/secured.json b0b0c04 > > ambari-server/src/test/resources/stacks/HDP/2.0.7/services/HDFS/metainfo.xml > 69f13e9 > > Diff: https://reviews.apache.org/r/32414/diff/ > > > Testing > ------- > > Manual testing to verify that topology script is configured and available, > services (HDFS, MAPRED2) are marked for restart when a change is made to host > rack information, and topology script can be invoked to get updated rack > information. > > New unit tests added. > > All unit tests pass. > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 33:36 min > [INFO] Finished at: 2015-03-23T15:47:10-04:00 > [INFO] Final Memory: 43M/600M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Tom Beerbower > >
