subject:"Review Request 45519\: AMBARI\-15637. BRANCH\-2.2 If RU\/EU is paused, services are restarted on the older version. EU is more complex since stopping services should use the original version."

Re: Review Request 45519: AMBARI-15637. BRANCH-2.2 If RU/EU is paused, services are restarted on the older version. EU is more complex since stopping services should use the original version.

2016-03-31 Thread Alejandro Fernandez

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

(Updated March 31, 2016, 10 p.m.)

Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan
Hurley, Jayush Luniya, and Nate Cole.

Changes
---

Addressed comments.

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 (updated)
-

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/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java
926807f

ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java
3ad2240

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

Re: Review Request 45519: AMBARI-15637. BRANCH-2.2 If RU/EU is paused, services are restarted on the older version. EU is more complex since stopping services should use the original version.

2016-03-31 Thread Jonathan Hurley


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


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)


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)


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)


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)


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

Re: Review Request 45519: AMBARI-15637. BRANCH-2.2 If RU/EU is paused, services are restarted on the older version. EU is more complex since stopping services should use the original version.

2016-03-31 Thread Alejandro Fernandez

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

(Updated March 31, 2016, 7:54 p.m.)