----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58092/#review170724 -----------------------------------------------------------
Ship it! Just the nit about the logging. I agree about not adding the extra work for being able to say once|always when scoping a server-side task. I just don't see why it would ever need to run more than once. They, then, should be scoped implicitely to only run once. ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java Lines 245 (patched) <https://reviews.apache.org/r/58092/#comment243610> Can we make this LOG.debug("", tw) to prevent the toString() when DEBUG is off? - Jonathan Hurley On March 30, 2017, 9:01 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58092/ > ----------------------------------------------------------- > > (Updated March 30, 2017, 9:01 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Dmitro Lisnichenko, and > Jonathan Hurley. > > > Bugs: AMBARI-20640 > https://issues.apache.org/jira/browse/AMBARI-20640 > > > Repository: ambari > > > Description > ------- > > Some server side actions, noteably config changes, should happen only one > time. there are a couple of cases: > > > <component name="MAPREDUCE2_CLIENT"> > <pre-upgrade> > <task xsi:type="server_action" > class="org.apache.ambari.server.serveraction.upgrades.FixLzoCodecPath"> > <summary>Verifying LZO codec path for mapreduce</summary> > </task> > </pre-upgrade> > </component> > > <component name="NODEMANAGER"> > <pre-upgrade> > <task xsi:type="configure" > id="hdp_2_5_0_0_add_spark2_yarn_shuffle"/> > </pre-upgrade> > > <pre-downgrade/> > > <upgrade> > <task xsi:type="restart-task" /> > </upgrade> > </component> > > These server-side actions are currently being scheduled for every instance of > MAPREDUCE2_CLIENT and NODEMANAGER. Luckily, they're idempotent so no harm is > being done, but it is unncessary scheduling of tasks. > > Originally I had intended to add another tag to the declaration to control > this behavior, but there actually isn't any use case that requires a > server-side action to happen for every host. That effort has been shelved > for a later time if it ever becomes necessary. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java > c939320a14 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapperBuilder.java > bd2bf14d81 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > 483880a1b1 > > ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java > 8e5ad0aadc > > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_multi_server_tasks.xml > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58092/diff/1/ > > > Testing > ------- > > Manual. Automated unit tests: > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 23:24.474s > [INFO] Finished at: Thu Mar 30 18:52:54 EDT 2017 > [INFO] Final Memory: 31M/301M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
