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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
 (line 2012)
<https://reviews.apache.org/r/45519/#comment189420>

    since this is now the "effective" version, maybe rename the variable as 
well.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
 (line 145)
<https://reviews.apache.org/r/45519/#comment189421>

    If they represent historic values, why not use the 
UpdateDesiredStackAction.class.getClassName() to build this Stirng. That way, 
someone renaming it won't miss this.



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 (line 1133)
<https://reviews.apache.org/r/45519/#comment189426>

    Why not just scope them to the current cluster from the DB?



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 (lines 1137 - 1146)
<https://reviews.apache.org/r/45519/#comment189427>

    Would a new NamedQuery that gets by clusterId, sorting by lastStartTime be 
easier here?



ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
 (lines 1191 - 1194)
<https://reviews.apache.org/r/45519/#comment189428>

    Missing break


- Jonathan Hurley


On March 31, 2016, 3:54 p.m., Alejandro Fernandez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45519/
> -----------------------------------------------------------
> 
> (Updated March 31, 2016, 3:54 p.m.)
> 
> 
> Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan 
> Hurley, Jayush Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15637
>     https://issues.apache.org/jira/browse/AMBARI-15637
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently, if RU/EU is paused, then restarting services manually will use the 
> version whose state is CURRENT. This means that services may be restarted on 
> the wrong version, preventing Ambari from correctly Finalizing the upgrade.
> Instead, the logic should be as follows during Upgrade:
> RU: always use to_version
> EU: if haven't completed the action "UPDATE_DESIRED_STACK_ID", then use the 
> from_version, otherwise, use the to_version.
> 
> During Downgrade, both should use the original version, which is actually the 
> to_version column in the upgrade table.
> 
> THIS CODE REVIEW IS FOR BRANCH-2.2, I WILL CREATE A DIFFERENT ONE FOR TRUNK.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java
>  1767b02 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  dd66dcc 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java
>  ec49364 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
>  ef8a8d4 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java
>  5c8b7f3 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java 
> 06f6ac1 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java
>  a12b204 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java
>  fd866a1 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> b49f566 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  3493508 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_skip_failures.xml
>  92b4fe3 
> 
> Diff: https://reviews.apache.org/r/45519/diff/
> 
> 
> Testing
> -------
> 
> Verified it worked
> 
> Assertions:
> A: restart a service (should have version parameter,
> B: run a service check (no version needed)
> C: run HDFS Rebalance (should have version parameter).
> 
> Test Cases:
> 1. Before stack upgrade, run A, B, and C, which should all use the current 
> version
> 2. EU, immediately click pause. Run A, B, and C, which should use the 
> original version if it exists
> 3. EU, after the services have been stopped and the stack has been upgraded. 
> Run A, B, and C, which should use the new version since the services are now 
> to be started.
> 4. EU, click downgrade and pause. Run A, B, C, which should use the original 
> version.
> 5. RU, click pause whenever a manual task occurs. Run A, B, and C, which 
> should use the destination version.
> 6. RU, click downgrade. Run A, B, and C, which should use the original 
> version.
> 
> Unit Tests passed,
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 01:10 h
> [INFO] Finished at: 2016-03-30T19:07:04-07:00
> [INFO] Final Memory: 130M/4052M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Alejandro Fernandez
> 
>

Reply via email to