----------------------------------------------------------- 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 > >
