You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Nate Cole <nc...@hortonworks.com> on 2017/03/31 01:01:48 UTC

Review Request 58092: Upgrade server-side actions should be performed only one time per group (ncole)

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58092/
-----------------------------------------------------------

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


Re: Review Request 58092: Upgrade server-side actions should be performed only one time per group (ncole)

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58092/#review170874
-----------------------------------------------------------


Ship it!




Ship It!

- Dmitro Lisnichenko


On March 31, 2017, 4:01 a.m., Nate Cole wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58092/
> -----------------------------------------------------------
> 
> (Updated March 31, 2017, 4:01 a.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
> 
>


Re: Review Request 58092: Upgrade server-side actions should be performed only one time per group (ncole)

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
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
> 
>