Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

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



It looks like this was pushed, is there a need to keep the review open?

- Nate Cole


On May 19, 2016, 11:23 a.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 19, 2016, 11:23 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1e040e6 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 6e27da6 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> d755516 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
>  dda1e7a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
>  15be8b4 
>   
> ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

> On May 19, 2016, 7:31 p.m., Jayush Luniya wrote:
> > Ship It!

Committed patch in trunk and branch-2.4


- Jayush


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


On May 19, 2016, 3:23 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 19, 2016, 3:23 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1e040e6 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 6e27da6 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> d755516 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
>  dda1e7a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
>  15be8b4 
>   
> ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
> 
> Diff: 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

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


Ship it!




Ship It!

- Jayush Luniya


On May 19, 2016, 3:23 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 19, 2016, 3:23 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1e040e6 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 6e27da6 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> d755516 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
>  dda1e7a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
>  15be8b4 
>   
> ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Alejandro Fernandez]

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


Ship it!




Ship It!

- Alejandro Fernandez


On May 19, 2016, 3:23 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 19, 2016, 3:23 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1e040e6 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 6e27da6 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> d755516 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
>  dda1e7a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
>  15be8b4 
>   
> ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far. 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

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

(Updated May 19, 2016, 3:23 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush Luniya, 
and Nate Cole.


Changes
---

Switched the  tag to be  and .  
This distinguishes between the two possible meanings the  tag used to 
have.  UpgradePackTest has a test to check that a new group can be added after 
an existing group and at the same time entries in the new group can be ordered 
using the  tag, if the same group is mentioned in 
several service upgrade xml files (although the actual test just adds the same 
group twice in a service upgrade xml file).


Bugs: AMBARI-15388
https://issues.apache.org/jira/browse/AMBARI-15388


Repository: ambari


Description
---

Currently the upgrade is defined as a series of xml files specific to the 
current stack version and the target stack version. Each upgrade xml defines 
the overall sequence of the upgrade and what needs to be done for each service. 
It would both easier to maintain and easier to add new services, if the 
services themselves could specify what should be done during their upgrade.

There are two ways to make these changes, the alternate approach would be to 
only make the java changes and not split the upgrade xml files.  This would 
still allow new services to add themselves into the upgrade.  The benefit of 
this is that for the stack services you only have one upgrade xml file.  The 
problem with that is it is easier for a particular service to have 
unintentional changes between upgrade xml files.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
 7f7a49e 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
 8a7b42b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
f781574 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
13d5047 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
5a18b3f 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 88f6e19 
  ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
43cefb9 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
 b860731 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
 3325469 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
 67d7fdb 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
 5cda422 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
6b74af0 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
9fb2bba 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
1e040e6 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
e3bc7a3 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
6e27da6 
  ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
d755516 
  
ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
 dda1e7a 
  
ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
 15be8b4 
  
ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
 PRE-CREATION 

Diff: https://reviews.apache.org/r/45169/diff/


Testing
---

Manual testing so far.  I have the code read the upgrade xml and all of its 
service specific xml files, built the upgrade pack and then write 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  line 844
> > 
> >
> > The after tag is overloaded. Meaning it could mean insert this service 
> > after another service in some place and in some places it could mean insert 
> > this  group after another group. I am wondering if there would be a case 
> > where we would need a combination (example: Create a new group that is not 
> > in the stack upgrade pack and add 2 new services to this group in a 
> > specific order). 
> > 
> > Instead of leaving the  tag to interpretation, why not be 
> > explicit as ,  etc. That way we can 
> > support combinations
> 
> Jayush Luniya wrote:
> Example:
> 
> 
>   CORE_MASTERS
>   
> MY_MASTER_1
>   
> 
> 
> 
> 
>   CORE_MASTERS
>   MY_SERVICE_1  
>   
> MY_MASTER_2
>   
> 
>  
>  Alterative
> 
>
>   CORE_MASTERS
>   
> MY_MASTER_1
>   
> 
> 
> 
> 
>   CORE_MASTERS
>   
> MY_SERVICE_1  
> MY_MASTER_2
>   
> 
> 
> Tim Thorpe wrote:
> With custom services they should just be able to create two new groups 
> rather than try to combine them into one.  If there really is a need for the 
> a new group that spans multiple services, then it is probably a candidate for 
> inclusion in the main upgrade xml.  You can even include an empty group in 
> the main upgrade xml like:
> 
> 
> 
> Jayush Luniya wrote:
> RE: If there really is a need for the a new group that spans multiple 
> services, then it is probably a candidate for inclusion in the main upgrade 
> xml.
> Well these services are add-on services meaning say not part of HDP stack 
> itself (example: HAWQ and PFX add-on services on HDP stack). So putting them 
> in stack upgrade pack wouldnt be right. 
> 
> Yes these can be in 2 separate groups, but that would be because of this 
> restriction.
> 
> Besides its also confusing when we overload the same property element. 
> For example the group name in upgrade pack could be the service name itself 
> like below, so  STORM could mean after service STORM or after 
> group STORM :) 
> 
> 
> https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml#L356-L364
> 
>   true
>   
> NIMBUS
> SUPERVISOR
> STORM_UI_SERVER
> DRPC_SERVER
>   
> 

Currently, I'm looping through all the groups from the main upgrade xml.  I add 
those to a  map.  This means the group from the main 
upgrade xml (if there was one) will be the first group in the list.  This is 
important because I know those groups are already internally ordered.  By that 
I mean there services/priorities/stages do NOT need to be shifted in the 
overall order (all insertions into that order are determined by the subsequent 
groups in the list).

Currently when I process the service upgade xml files, for each group if the 
group name already exists in the map then this means the group existed in the 
main upgrade xml and the entries (services/priorities/stages) will be inserted 
into the original entry order based on the  tag.  If the group name 
doesn't exist, it is added with its group as the first group in the list.  In 
this case I never expect another group with the same name to be added to that 
list.

The missing piece is with this change I need to reorder the List in 
the map to find a group that doesn't have an  and if all 
groups do have that tag, then I guess I have an error situation.  Adding 
explicit tags for  and  would make it 
easier to understand.  I would prefer to limit it to two tags  
and  rather than split the  into 
separate tags for services/priorities/stages.


- Tim


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


On May 18, 2016, 6:26 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 18, 2016, 6:26 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  line 844
> > 
> >
> > The after tag is overloaded. Meaning it could mean insert this service 
> > after another service in some place and in some places it could mean insert 
> > this  group after another group. I am wondering if there would be a case 
> > where we would need a combination (example: Create a new group that is not 
> > in the stack upgrade pack and add 2 new services to this group in a 
> > specific order). 
> > 
> > Instead of leaving the  tag to interpretation, why not be 
> > explicit as ,  etc. That way we can 
> > support combinations
> 
> Jayush Luniya wrote:
> Example:
> 
> 
>   CORE_MASTERS
>   
> MY_MASTER_1
>   
> 
> 
> 
> 
>   CORE_MASTERS
>   MY_SERVICE_1  
>   
> MY_MASTER_2
>   
> 
>  
>  Alterative
> 
>
>   CORE_MASTERS
>   
> MY_MASTER_1
>   
> 
> 
> 
> 
>   CORE_MASTERS
>   
> MY_SERVICE_1  
> MY_MASTER_2
>   
> 
> 
> Tim Thorpe wrote:
> With custom services they should just be able to create two new groups 
> rather than try to combine them into one.  If there really is a need for the 
> a new group that spans multiple services, then it is probably a candidate for 
> inclusion in the main upgrade xml.  You can even include an empty group in 
> the main upgrade xml like:
> 
> 

RE: If there really is a need for the a new group that spans multiple services, 
then it is probably a candidate for inclusion in the main upgrade xml.
Well these services are add-on services meaning say not part of HDP stack 
itself (example: HAWQ and PFX add-on services on HDP stack). So putting them in 
stack upgrade pack wouldnt be right. 

Yes these can be in 2 separate groups, but that would be because of this 
restriction.

Besides its also confusing when we overload the same property element. For 
example the group name in upgrade pack could be the service name itself like 
below, so  STORM could mean after service STORM or after group 
STORM :) 

https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml#L356-L364

  true
  
NIMBUS
SUPERVISOR
STORM_UI_SERVER
DRPC_SERVER
  



- Jayush


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


On May 18, 2016, 6:26 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 18, 2016, 6:26 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

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

(Updated May 18, 2016, 6:26 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush Luniya, 
and Nate Cole.


Changes
---

Fixing issue exposed by UpgradeResourceProviderHDP22Test.  Switched the target 
version in src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml 
to 2.4


Bugs: AMBARI-15388
https://issues.apache.org/jira/browse/AMBARI-15388


Repository: ambari


Description
---

Currently the upgrade is defined as a series of xml files specific to the 
current stack version and the target stack version. Each upgrade xml defines 
the overall sequence of the upgrade and what needs to be done for each service. 
It would both easier to maintain and easier to add new services, if the 
services themselves could specify what should be done during their upgrade.

There are two ways to make these changes, the alternate approach would be to 
only make the java changes and not split the upgrade xml files.  This would 
still allow new services to add themselves into the upgrade.  The benefit of 
this is that for the stack services you only have one upgrade xml file.  The 
problem with that is it is easier for a particular service to have 
unintentional changes between upgrade xml files.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
 7f7a49e 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
 8a7b42b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
f781574 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
13d5047 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
5a18b3f 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 88f6e19 
  ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
43cefb9 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
 b860731 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
 3325469 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
 67d7fdb 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
 5cda422 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
6b74af0 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
9fb2bba 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
1e040e6 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
e3bc7a3 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
6e27da6 
  ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
d755516 
  
ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
 dda1e7a 
  
ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
 15be8b4 
  
ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml
 PRE-CREATION 

Diff: https://reviews.apache.org/r/45169/diff/


Testing
---

Manual testing so far.  I have the code read the upgrade xml and all of its 
service specific xml files, built the upgrade pack and then write the full 
upgrade xml to disk and then compare the results to the original upgrade xml.

This review is mostly for the design doc which is attached to the JIRA.  Not 
sure how to create a review board with a design doc instead of a patch file.


Thanks,

Tim Thorpe

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

> On May 17, 2016, 10:30 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  lines 890-892
> > 
> >
> > Really need to throw here or just log it and continue; ?
> 
> Tim Thorpe wrote:
> When the StackManager encounters errors while reading the stack, it 
> almost always ends up throwing an exception and ambari-server fails to start. 
>  I'm ok with just logging the issue for this and other errors but then you 
> could think that everything is fine with the service specific upgrade xml 
> unless you dig through the log.
> 
> Nate Cole wrote:
> That's true, but in this case you aren't bound by reading the stack per 
> se, you're acting on the applicability of an XML element.  This is happening 
> in mergeServiceCheckGrouping, implying you're only considering 
> ServiceCheckGroupings anyway.
> 
> If we actually used XSD (I have a future work-item for that) then sure, 
> that could cause failure of stack.  But in this case, I think a simple no-op 
> continue; is fine.
> 
> Tim Thorpe wrote:
> Jayush suggested that I needed to make sure when merging groups that all 
> the groups were of the same type.  So this applies to regular Grouping, 
> ClusterGrouping, ServiceCheckGrouping, etc...
> 
>   List list = allGroupMap.get(group.name);
>   if (list.size() > 0) {
> Grouping first = list.get(0);
> if (!first.getClass().equals(group.getClass())) {
>   throw new AmbariException("Expected class: " + 
> first.getClass() + " instead of " + group.getClass());
> }
>   }
>   allGroupMap.get(group.name).add(group);
> 
> Basically with the above code, the logging would never occur when merging 
> ServiceCheckGroupings.  Do you think the above code should just log as well 
> or is it more critical because it applies to groups other than 
> ServiceCheckGroupings?

Is that a bit restrictive?  It means you can't add a ServiceCheckGrouping right 
after a "regular" Grouping?  I might be missing the point.


- Nate


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


On May 18, 2016, 9:19 a.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 18, 2016, 9:19 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 2:30 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  lines 890-892
> > 
> >
> > Really need to throw here or just log it and continue; ?
> 
> Tim Thorpe wrote:
> When the StackManager encounters errors while reading the stack, it 
> almost always ends up throwing an exception and ambari-server fails to start. 
>  I'm ok with just logging the issue for this and other errors but then you 
> could think that everything is fine with the service specific upgrade xml 
> unless you dig through the log.
> 
> Nate Cole wrote:
> That's true, but in this case you aren't bound by reading the stack per 
> se, you're acting on the applicability of an XML element.  This is happening 
> in mergeServiceCheckGrouping, implying you're only considering 
> ServiceCheckGroupings anyway.
> 
> If we actually used XSD (I have a future work-item for that) then sure, 
> that could cause failure of stack.  But in this case, I think a simple no-op 
> continue; is fine.

Jayush suggested that I needed to make sure when merging groups that all the 
groups were of the same type.  So this applies to regular Grouping, 
ClusterGrouping, ServiceCheckGrouping, etc...

  List list = allGroupMap.get(group.name);
  if (list.size() > 0) {
Grouping first = list.get(0);
if (!first.getClass().equals(group.getClass())) {
  throw new AmbariException("Expected class: " + first.getClass() + 
" instead of " + group.getClass());
}
  }
  allGroupMap.get(group.name).add(group);

Basically with the above code, the logging would never occur when merging 
ServiceCheckGroupings.  Do you think the above code should just log as well or 
is it more critical because it applies to groups other than 
ServiceCheckGroupings?


- Tim


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


On May 18, 2016, 1:19 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 18, 2016, 1:19 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

> On May 17, 2016, 10:30 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  lines 890-892
> > 
> >
> > Really need to throw here or just log it and continue; ?
> 
> Tim Thorpe wrote:
> When the StackManager encounters errors while reading the stack, it 
> almost always ends up throwing an exception and ambari-server fails to start. 
>  I'm ok with just logging the issue for this and other errors but then you 
> could think that everything is fine with the service specific upgrade xml 
> unless you dig through the log.

That's true, but in this case you aren't bound by reading the stack per se, 
you're acting on the applicability of an XML element.  This is happening in 
mergeServiceCheckGrouping, implying you're only considering 
ServiceCheckGroupings anyway.

If we actually used XSD (I have a future work-item for that) then sure, that 
could cause failure of stack.  But in this case, I think a simple no-op 
continue; is fine.


- Nate


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


On May 18, 2016, 9:19 a.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 18, 2016, 9:19 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 5a18b3f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
>  3325469 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1e040e6 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 6e27da6 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> d755516 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
>  dda1e7a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
>  15be8b4 
>   
> ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
>  PRE-CREATION 
>   
> ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
>  PRE-CREATION 
>   
> 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

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

(Updated May 18, 2016, 1:19 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush Luniya, 
and Nate Cole.


Changes
---

The new patch still has the following remaining items that need answers:

1) It still throws exceptions rather than logging issues.  This is consistent 
with how the StackManager deals with things in the stack that don't work.  
Waiting for Nate's response on the issue he opened.

2) The after tag still is multi purpose.  It orders a new group among the 
original groups and a new stage/priority/service among the existing ones in an 
original group.  The only unsupported use cases is to add a new group in a 
service upgrade xml file that will be added to in another service's upgrade xml 
file.  The correct approach would be to use different groups OR to add the 
group in the stack upgrade xml file.  Waiting for Jayush's response on that.


Bugs: AMBARI-15388
https://issues.apache.org/jira/browse/AMBARI-15388


Repository: ambari


Description
---

Currently the upgrade is defined as a series of xml files specific to the 
current stack version and the target stack version. Each upgrade xml defines 
the overall sequence of the upgrade and what needs to be done for each service. 
It would both easier to maintain and easier to add new services, if the 
services themselves could specify what should be done during their upgrade.

There are two ways to make these changes, the alternate approach would be to 
only make the java changes and not split the upgrade xml files.  This would 
still allow new services to add themselves into the upgrade.  The benefit of 
this is that for the stack services you only have one upgrade xml file.  The 
problem with that is it is easier for a particular service to have 
unintentional changes between upgrade xml files.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
 7f7a49e 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
 8a7b42b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
f781574 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
13d5047 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
5a18b3f 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 88f6e19 
  ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
43cefb9 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
 b860731 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java
 3325469 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
 67d7fdb 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
 5cda422 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
6b74af0 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
9fb2bba 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
1e040e6 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
e3bc7a3 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
6e27da6 
  ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
d755516 
  
ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java
 dda1e7a 
  
ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java
 15be8b4 
  
ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml
 PRE-CREATION 
  
ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml
 PRE-CREATION 
  

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

> On May 17, 2016, 2:23 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, 
> > line 166
> > 
> >
> > Does the group name have to be unique now or is this only for the 
> > purposes of making debugging easier?
> 
> Tim Thorpe wrote:
> In order to be able to add a custom component into the priority list of a 
> particular service check step, there would need to be something distinct 
> about them, either the name or the title.
> 
> Tim Thorpe wrote:
> Currently the group ordering logic is counting on the group name to be 
> unique.  It could use the group title.

The title is used directly by the UI and should not be a reliable indicator of 
group.


- Nate


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


On May 16, 2016, 2:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 2:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  line 829
> > 
> >
> > What about RestartGrouping, ColocatedGrouping, StartGrouping, 
> > StopGrouping, UpdateStackGrouping? How about parent.merge(iterator) instead?

Implemented parent.merge(iterator), RestartGrouping, ColocatedGrouping, 
StartGrouping, StopGrouping, UpdateStackGrouping just all use the default 
behavior like what was happening before with mergeRegularGrouping.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 2:30 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  lines 822-831
> > 
> >
> > Visitor or abstract method?

Added the mergeRegularGroupingByName to the Grouping class as 
merge(Iterator) throws AmbariException
The other methods were added to ClusterGrouping and ServiceCheckGrouping 
respectively, overridding the merge method from the Grouping class.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 6:23 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java,
> >  line 485
> > 
> >
> > May want to check that upgradeFile is not null before calling 
> > getAbsolutePath() on it, or return null if upgradeFile is null at the start.

This should never be null because of how the method is called.  A list of files 
in the upgrade folder is processed and the one that matches config-upgrade.xml 
is parsed.  But I added the extra check just in case.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote:
> > Can you add unit test coverage?
> 
> Jayush Luniya wrote:
> We should have unit tests in particular to validate incorrectly authored 
> service upgrade packs. What happens if we add a circular dependency (example: 
> KAFKA is marked with KNOX and KNOX is marked with 
> KAFKA)
> 
> Jayush Luniya wrote:
> Also we should document any unsupported use cases.

Added unit test for both a possible case and for the circular dependency.  
Really the circular dependency case also covers when you have FOO after BAR but 
BAR is not included.  Where should the documentation go?


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 6:23 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, 
> > line 166
> > 
> >
> > Does the group name have to be unique now or is this only for the 
> > purposes of making debugging easier?

In order to be able to add a custom component into the priority list of a 
particular service check step, there would need to be something distinct about 
them, either the name or the title.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Alejandro Fernandez]

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




ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
(line 473)


May want to check that upgradeFile is not null before calling 
getAbsolutePath() on it, or return null if upgradeFile is null at the start.



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 702)


Add some java doc to these new methods.



ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml (line 
166)


Does the group name have to be unique now or is this only for the purposes 
of making debugging easier?


- Alejandro Fernandez


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  line 844
> > 
> >
> > The after tag is overloaded. Meaning it could mean insert this service 
> > after another service in some place and in some places it could mean insert 
> > this  group after another group. I am wondering if there would be a case 
> > where we would need a combination (example: Create a new group that is not 
> > in the stack upgrade pack and add 2 new services to this group in a 
> > specific order). 
> > 
> > Instead of leaving the  tag to interpretation, why not be 
> > explicit as ,  etc. That way we can 
> > support combinations

Example:


  CORE_MASTERS
  
MY_MASTER_1
  




  CORE_MASTERS
  MY_SERVICE_1  
  
MY_MASTER_2
  

 
 Alterative

   
  CORE_MASTERS
  
MY_MASTER_1
  




  CORE_MASTERS
  
MY_SERVICE_1  
MY_MASTER_2
  



- Jayush


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

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




ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 674)


new ArrayList<>()



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 705)


Use constant instead
See StackDefinitionDirectory::CONFIG_UPGRADE_XML_FILENAME_PREFIX



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 734)


new ArrayList<>()



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 745)


Validate that all sub-groups in the list are of the same type.



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 753)


Might not necessarily be original if this is a new group for the service. 
(Line#745).



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 829)


What about RestartGrouping, ColocatedGrouping, StartGrouping, StopGrouping, 
UpdateStackGrouping? How about parent.merge(iterator) instead?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 837)


nit: child instead of next?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 844)


The after tag is overloaded. Meaning it could mean insert this service 
after another service in some place and in some places it could mean insert 
this  group after another group. I am wondering if there would be a case where 
we would need a combination (example: Create a new group that is not in the 
stack upgrade pack and add 2 new services to this group in a specific order). 

Instead of leaving the  tag to interpretation, why not be explicit 
as ,  etc. That way we can support 
combinations



ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml (line 
158)


SERVICE_CHECK_1 instead


- Jayush Luniya


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

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




ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 743)


nit: can use newer collections syntax without the generic specifics: 
List list = new ArrayList<>();



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 750)


nit: can use newer collections syntax without the generic specifics: 
Map map = new HashMap<>();



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 764)


could use a MultiMap?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(lines 765 - 766)


An Entry iterator is preferred



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(lines 822 - 831)


Visitor or abstract method?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(lines 836 - 837)


Need an iterator directly?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(lines 890 - 892)


Really need to throw here or just log it and continue; ?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 934)


Some javadoc would be nice



ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 (lines 112 - 113)


With no other exceptions, can just use "regular" logging syntax:  
LOG.debug("Service upgrades folder {} for service {} in stack {} is empty.", 
absUpgradesDir, serviceDir.getName(), stackId)



ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 (lines 116 - 117)


Maybe LOG.info() here.  If nothing happens we'll have no record unless 
DEBUG is enabled (which is, admittedly, an awful idea)



ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml (line 
166)


Changing this name may (or may not) impact UI.  The group name is not 
guaranteed unique, and some additional information may be displayed based on 
it's name (unconfirmed, but that's the intent).  Also, make consistent using an 
'_': SERVICE_CHECK_1


- Nate Cole


On May 16, 2016, 2:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 2:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On May 17, 2016, 6:41 a.m., Jayush Luniya wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java,
> >  line 685
> > 
> >
> > UGM?

This was just for testing purposes.  I'll remove the unnecessary logging.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote:
> > Can you add unit test coverage?

We should have unit tests in particular to validate incorrectly authored 
service upgrade packs. What happens if we add a circular dependency (example: 
KAFKA is marked with KNOX and KNOX is marked with 
KAFKA)


- Jayush


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

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




ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
(line 685)


UGM?


- Jayush Luniya


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> ---
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

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

(Updated May 13, 2016, 1:46 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate Cole.


Changes
---

Added javadoc for UpgradePack changes


Bugs: AMBARI-15388
https://issues.apache.org/jira/browse/AMBARI-15388


Repository: ambari


Description
---

Currently the upgrade is defined as a series of xml files specific to the 
current stack version and the target stack version. Each upgrade xml defines 
the overall sequence of the upgrade and what needs to be done for each service. 
It would both easier to maintain and easier to add new services, if the 
services themselves could specify what should be done during their upgrade.

There are two ways to make these changes, the alternate approach would be to 
only make the java changes and not split the upgrade xml files.  This would 
still allow new services to add themselves into the upgrade.  The benefit of 
this is that for the stack services you only have one upgrade xml file.  The 
problem with that is it is easier for a particular service to have 
unintentional changes between upgrade xml files.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
 7f7a49e 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
 8a7b42b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
f781574 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
13d5047 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
6129ec0 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
 88f6e19 
  ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
43cefb9 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
 b860731 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
 67d7fdb 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
 5cda422 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
6b74af0 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
9fb2bba 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
1cd2ffa 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
e3bc7a3 
  ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
9c6a02d 
  ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
1745de8 

Diff: https://reviews.apache.org/r/45169/diff/


Testing
---

Manual testing so far.  I have the code read the upgrade xml and all of its 
service specific xml files, built the upgrade pack and then write the full 
upgrade xml to disk and then compare the results to the original upgrade xml.

This review is mostly for the design doc which is attached to the JIRA.  Not 
sure how to create a review board with a design doc instead of a patch file.


Thanks,

Tim Thorpe

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On March 29, 2016, 12:49 p.m., Nate Cole wrote:
> > I think you need a more concrete way of ordering here.  What if two 
> > services are marked as  YARN?  Which one takes precedence?  You may 
> > want to introduce an  in order to 
> > specifically state how it happens.  Order would be a float, such that if we 
> > have a 1.0 and a 2.0, and some service goes in between, then we don't need 
> > to reorder the world (just use 1.5, or 1.1 or whatever).
> > 
> > In addition, if you are making separate files, it's conceivable that config 
> > changes for services can go directly in their respective files.  We broke 
> > them up to separate church-and-state, if you will.
> > 
> > It seems like a more achievable goal might be to allow services to announce 
> > how they fit into how the upgrade packs work today, without rewriting how 
> > the whole system works.
> 
> Jayush Luniya wrote:
> RE: It seems like a more achievable goal might be to allow services to 
> announce how they fit into how the upgrade packs work today, without 
> rewriting how the whole system works.
> +1 on allowing add-on/custom service to extend stack upgrade packs 
> instead of breaking down the entire upgrade pack to service level.
> 
> Tim Thorpe wrote:
> We decided we wouldn't split the upgrade xml into pieces for all the 
> stack services.  Instead the goal will be limited to allow custom services to 
> integrate into the upgrade process by providing their own service upgrade xml 
> which will define their prereqs, order and process.  This means we still need 
> a way for a service to indicate when during the process each of its steps 
> needs to be done.
> 
> I am a bit leary of the order number because there are actually two ways 
> that service steps need to get added.  (In the example below I will still use 
> core services to demonstrate).
> 
> The first case is where a service needs to get integrated into an exists 
> step.  In the example below, a service (HBASE) needs to be backed up prior to 
> upgrade:
> 
> 
>   KNOX
>   
> 
>   scripts/hbase_upgrade.py
>   take_snapshot
> 
>   
> 
> 
> The second case is where a service has a new step which just needs to be 
> included in the order.  In the example below, a service (KNOX) needs a new 
> step added:
> 
> 
>   KAFKA
>   true
>   
> KNOX_GATEWAY
>   
> 
> 
> Nate proposed using an order number but this gets messy in the case of 
> integrating into an existing step.  There would be 2 order numbers, one for 
> the group and the other for the execute-stage:
> 
> 
>   
> 
>   scripts/hbase_upgrade.py
>   take_snapshot
> 
>   
> 
> 
> This seems a bit confusing.  It would also require changes to the actual 
> upgrade xml when now we would otherwise not require any changes to those 
> files.  Using a tag like , does allow for some ambiguity in what the 
> actual order will be if several services specify that they should come after 
> the same step or service.  I'm not convinced that it would really matter what 
> the order is.  Yes, if a given service needs to be processed after another 
> then the order does matter but in that case the  tag should be changed 
> to reflect that need.  If we have two custom services that both need to be 
> started after HBASE then it shouldn't matter which one gets started before 
> the other.  If there are dependencies between those two custom services then 
> one should state it should be started after the other.
> 
> Nate Cole wrote:
> You could use order only at the group level.  I wouldn't expect that you 
> would be "injecting" execute-stage into an existing / 
> as those are already-vetted actions.  There's no reason that two groups in a 
> row can't work against the same service/component.  You're right that most 
> cases the order doesn't matter, but the design should allow for it.

Hi Nate, There are several areas were order maybe needed: 

1) ordering check elements within the prerequisite-checks tag
2) ordering group elements within the order tag
3) ordering execute-stage elements within a particular group tag (such as 
within prepare backups)
4) ordering service elements within a particular group tag (such as within the 
core slaves section)
5) ordering service elements within a priority tag (for service checks)

I can see using an order attribute within the group elements but adding an 
order attribute to the execute-stage and service elements would get confusing.  
If only custom services use the separate service specific upgrade xml, then 
adding all this information to the main upgrade xml just adds clutter and 
confusion.  If I am the developer of a custom service and my service depends on 
HIVE and requires a backup prior in the 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Nate Cole]

> On March 29, 2016, 8:49 a.m., Nate Cole wrote:
> > I think you need a more concrete way of ordering here.  What if two 
> > services are marked as  YARN?  Which one takes precedence?  You may 
> > want to introduce an  in order to 
> > specifically state how it happens.  Order would be a float, such that if we 
> > have a 1.0 and a 2.0, and some service goes in between, then we don't need 
> > to reorder the world (just use 1.5, or 1.1 or whatever).
> > 
> > In addition, if you are making separate files, it's conceivable that config 
> > changes for services can go directly in their respective files.  We broke 
> > them up to separate church-and-state, if you will.
> > 
> > It seems like a more achievable goal might be to allow services to announce 
> > how they fit into how the upgrade packs work today, without rewriting how 
> > the whole system works.
> 
> Jayush Luniya wrote:
> RE: It seems like a more achievable goal might be to allow services to 
> announce how they fit into how the upgrade packs work today, without 
> rewriting how the whole system works.
> +1 on allowing add-on/custom service to extend stack upgrade packs 
> instead of breaking down the entire upgrade pack to service level.
> 
> Tim Thorpe wrote:
> We decided we wouldn't split the upgrade xml into pieces for all the 
> stack services.  Instead the goal will be limited to allow custom services to 
> integrate into the upgrade process by providing their own service upgrade xml 
> which will define their prereqs, order and process.  This means we still need 
> a way for a service to indicate when during the process each of its steps 
> needs to be done.
> 
> I am a bit leary of the order number because there are actually two ways 
> that service steps need to get added.  (In the example below I will still use 
> core services to demonstrate).
> 
> The first case is where a service needs to get integrated into an exists 
> step.  In the example below, a service (HBASE) needs to be backed up prior to 
> upgrade:
> 
> 
>   KNOX
>   
> 
>   scripts/hbase_upgrade.py
>   take_snapshot
> 
>   
> 
> 
> The second case is where a service has a new step which just needs to be 
> included in the order.  In the example below, a service (KNOX) needs a new 
> step added:
> 
> 
>   KAFKA
>   true
>   
> KNOX_GATEWAY
>   
> 
> 
> Nate proposed using an order number but this gets messy in the case of 
> integrating into an existing step.  There would be 2 order numbers, one for 
> the group and the other for the execute-stage:
> 
> 
>   
> 
>   scripts/hbase_upgrade.py
>   take_snapshot
> 
>   
> 
> 
> This seems a bit confusing.  It would also require changes to the actual 
> upgrade xml when now we would otherwise not require any changes to those 
> files.  Using a tag like , does allow for some ambiguity in what the 
> actual order will be if several services specify that they should come after 
> the same step or service.  I'm not convinced that it would really matter what 
> the order is.  Yes, if a given service needs to be processed after another 
> then the order does matter but in that case the  tag should be changed 
> to reflect that need.  If we have two custom services that both need to be 
> started after HBASE then it shouldn't matter which one gets started before 
> the other.  If there are dependencies between those two custom services then 
> one should state it should be started after the other.

You could use order only at the group level.  I wouldn't expect that you would 
be "injecting" execute-stage into an existing / as those 
are already-vetted actions.  There's no reason that two groups in a row can't 
work against the same service/component.  You're right that most cases the 
order doesn't matter, but the design should allow for it.


- Nate


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


On March 22, 2016, 2:40 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated March 22, 2016, 2:40 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate 
> Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Jayush Luniya]

> On March 29, 2016, 12:49 p.m., Nate Cole wrote:
> > I think you need a more concrete way of ordering here.  What if two 
> > services are marked as  YARN?  Which one takes precedence?  You may 
> > want to introduce an  in order to 
> > specifically state how it happens.  Order would be a float, such that if we 
> > have a 1.0 and a 2.0, and some service goes in between, then we don't need 
> > to reorder the world (just use 1.5, or 1.1 or whatever).
> > 
> > In addition, if you are making separate files, it's conceivable that config 
> > changes for services can go directly in their respective files.  We broke 
> > them up to separate church-and-state, if you will.
> > 
> > It seems like a more achievable goal might be to allow services to announce 
> > how they fit into how the upgrade packs work today, without rewriting how 
> > the whole system works.

RE: It seems like a more achievable goal might be to allow services to announce 
how they fit into how the upgrade packs work today, without rewriting how the 
whole system works.
+1 on allowing add-on/custom service to extend stack upgrade packs instead of 
breaking down the entire upgrade pack to service level.


- Jayush


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


On March 22, 2016, 6:40 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated March 22, 2016, 6:40 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate 
> Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java
>  9e2f997 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  c552e41 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml 
> 440bd50 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/falcon.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hbase.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hdfs.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hive.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/kafka.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/knox.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/oozie.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/ranger.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/storm.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/tez.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/yarn.xml 
> PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml
>  430114b 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/falcon.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/flume.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hbase.xml
>  PRE-CREATION 
>   
> 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On March 29, 2016, 12:49 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java,
> >  lines 535-547
> > 
> >
> > What exactly is the use case for marshalling back to the filesystem?  
> > Filesystem access is read-only when running Ambari as non-root, so this 
> > code will not work.  If you are looking for merge-ordering or something to 
> > make an "uber upgrade pack", then that can be done at orchestration or keep 
> > it in-memory.

Hi Nate, the purpose was really for testing.  I marshalled the results file 
back to the FS so I could compare the results with the original upgrade xml.


- Tim


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


On March 22, 2016, 6:40 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated March 22, 2016, 6:40 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate 
> Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java
>  9e2f997 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  c552e41 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml 
> 440bd50 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/falcon.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hbase.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hdfs.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hive.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/kafka.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/knox.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/oozie.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/ranger.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/storm.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/tez.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/yarn.xml 
> PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml
>  430114b 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/falcon.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/flume.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hbase.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hdfs.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hive.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/kafka.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/knox.xml
>  PRE-CREATION 
>   
> 

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

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

(Updated March 22, 2016, 6:40 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate Cole.


Bugs: AMBARI-15388
https://issues.apache.org/jira/browse/AMBARI-15388


Repository: ambari


Description
---

Currently the upgrade is defined as a series of xml files specific to the 
current stack version and the target stack version. Each upgrade xml defines 
the overall sequence of the upgrade and what needs to be done for each service. 
It would both easier to maintain and easier to add new services, if the 
services themselves could specify what should be done during their upgrade.

There are two ways to make these changes, the alternate approach would be to 
only make the java changes and not split the upgrade xml files.  This would 
still allow new services to add themselves into the upgrade.  The benefit of 
this is that for the stack services you only have one upgrade xml file.  The 
problem with that is it is easier for a particular service to have 
unintentional changes between upgrade xml files.


Diffs
-

  
ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java
 9e2f997 
  
ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 
c552e41 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
 b860731 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
 67d7fdb 
  
ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
 5cda422 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml 
440bd50 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/falcon.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hbase.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hdfs.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hive.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/kafka.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/knox.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/oozie.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/ranger.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/storm.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/tez.xml 
PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/yarn.xml 
PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml
 430114b 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/falcon.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/flume.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hbase.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hdfs.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hive.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/kafka.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/knox.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/oozie.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/pig.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/ranger.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/slider.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/spark.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/sqoop.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/storm.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/tez.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/yarn.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/zookeeper.xml
 PRE-CREATION 
  
ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.3.xml
 1b6cb8b 
  

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

[Tim Thorpe]

> On March 22, 2016, 6:27 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java,
> >  line 31
> > 
> >
> > Please give me some time to go through the design review and code 
> > review.
> > 
> > Thank you,
> > Alejandro

No rush.  Thanks for looking into it.


- Tim


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


On March 22, 2016, 6:18 p.m., Tim Thorpe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> ---
> 
> (Updated March 22, 2016, 6:18 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate 
> Cole.
> 
> 
> Bugs: AMBARI-15388
> https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java
>  9e2f997 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  c552e41 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml 
> 440bd50 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/falcon.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hbase.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hdfs.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/hive.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/kafka.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/knox.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/oozie.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/ranger.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/storm.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/tez.xml 
> PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/configs/yarn.xml 
> PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2.xml
>  430114b 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/falcon.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/flume.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hbase.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hdfs.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/hive.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/kafka.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/knox.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/oozie.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/pig.xml
>  PRE-CREATION 
>   
> ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/nonrolling-upgrade-2.2/ranger.xml
>  PRE-CREATION 
>   
>