You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Dmitro Lisnichenko <dl...@hortonworks.com> on 2015/08/27 17:08:42 UTC

Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

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

(Updated Aug. 27, 2015, 3:08 p.m.)


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


Changes
-------

Since it going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)


Summary (updated)
-----------------

[Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack


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


Repository: ambari


Description (updated)
-------

The configs need to move out of the Upgrade Packs and into their own file.
This will make it easier to maintain, and clearer since there will not be any dups.

Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
  ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
  ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
  ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
  ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 

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


Testing (updated)
-------

just published preview of changes


Thanks,

Dmitro Lisnichenko


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java, line 49
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056027#file1056027line49>
> >
> >     Can you switch this to fully qualified imports?

ok


> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java, line 429
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056029#file1056029line429>
> >
> >     I'm ok with this convention. Can we make the comparison case insensitive?

good catch


> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, line 207
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056032#file1056032line207>
> >
> >     Does initializing the ArrayList require passing in the type as <UpgradeGroupHolder>?

Since Java 1.7, Diamond operator allows compiler to infer generic type


> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, line 235
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056032#file1056032line235>
> >
> >     Same comment about type in the ArrayList here.

the same


> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java, line 151
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056034#file1056034line151>
> >
> >     Why did the XmlElement names have to be removed?

Since we move all configuration changes to a separate file (`upgrade-config-changes-*.xml`), configure task definition is not going to be supported anymore in upgrade pack. We generate relevant configure tasks in runtime. Moreover, in responce to use cases mentioned by Jonathan, I'm going to add per-service config change priority.


> On Aug. 27, 2015, 5:07 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml, line 19
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056040#file1056040line19>
> >
> >     Great, this is the same structure I had in mind.
> >     We'll need to be careful when merging this to branch-2.1 and trunk so that we don't forget any properties.
> >     For now, we only need to verify it works for core services (HDFS, YARN, MR, HBASE, ZK).

good point


- Dmitro


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


On Aug. 27, 2015, 3:08 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 3:08 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review96699
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java (line 49)
<https://reviews.apache.org/r/37682/#comment152321>

    Can you switch this to fully qualified imports?



ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java (line 429)
<https://reviews.apache.org/r/37682/#comment152320>

    I'm ok with this convention. Can we make the comparison case insensitive?



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (line 207)
<https://reviews.apache.org/r/37682/#comment152322>

    Does initializing the ArrayList require passing in the type as <UpgradeGroupHolder>?



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (line 235)
<https://reviews.apache.org/r/37682/#comment152323>

    Same comment about type in the ArrayList here.



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (line 249)
<https://reviews.apache.org/r/37682/#comment152327>

    This seems ok.



ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 
<https://reviews.apache.org/r/37682/#comment152328>

    Why did the XmlElement names have to be removed?



ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java (line 77)
<https://reviews.apache.org/r/37682/#comment152330>

    TODO.



ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml (line 19)
<https://reviews.apache.org/r/37682/#comment152331>

    Great, this is the same structure I had in mind.
    We'll need to be careful when merging this to branch-2.1 and trunk so that we don't forget any properties.
    For now, we only need to verify it works for core services (HDFS, YARN, MR, HBASE, ZK).


Looks good Dmitry.

- Alejandro Fernandez


On Aug. 27, 2015, 3:08 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 3:08 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Sept. 8, 2015, 5:54 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, line 282
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065308#file1065308line282>
> >
> >     Why was <Task> removed?

This is a new JDK 7 syntax sugar (the diamond operator)


> On Sept. 8, 2015, 5:54 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, line 651
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065301#file1065301line651>
> >
> >     FYI, for STW Upgrade, we'll need to apply configs after all services have been stopped, and before calling hdp-select set all.
> >     
> >     For now, I'm ok with just getting this patch in.

With my patch, config changes are applied when configure relevant configure tasks are executed. So the order is fully customizable when writing down upgrade pack.


> On Sept. 8, 2015, 5:54 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, line 624
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065301#file1065301line624>
> >
> >     This function is allowed to return null, let's make sure the value isn't null before we pass it to another function.

Good catch


> On Sept. 8, 2015, 5:54 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, line 231
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065308#file1065308line231>
> >
> >     Why was <UpgradePack.OrderService> removed?

This is a new JDK 7 syntax sugar (the diamond operator)


> On Sept. 8, 2015, 5:54 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java, line 408
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065311#file1065311line408>
> >
> >     Let's use isEmpty or isBlank instead.

ok


- Dmitro


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


On Sept. 8, 2015, 3:37 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2015, 3:37 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review98029
-----------------------------------------------------------

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java (line 624)
<https://reviews.apache.org/r/37682/#comment154177>

    This function is allowed to return null, let's make sure the value isn't null before we pass it to another function.



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java (line 651)
<https://reviews.apache.org/r/37682/#comment154176>

    FYI, for STW Upgrade, we'll need to apply configs after all services have been stopped, and before calling hdp-select set all.
    
    For now, I'm ok with just getting this patch in.



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (line 231)
<https://reviews.apache.org/r/37682/#comment154179>

    Why was <UpgradePack.OrderService> removed?



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (line 282)
<https://reviews.apache.org/r/37682/#comment154178>

    Why was <Task> removed?



ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java (line 151)
<https://reviews.apache.org/r/37682/#comment154182>

    Let's use isEmpty or isBlank instead.



ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java (line 375)
<https://reviews.apache.org/r/37682/#comment154183>

    For the sake of getting this in, let's ignore unit tests for now.


- Alejandro Fernandez


On Sept. 8, 2015, 3:37 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2015, 3:37 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review98188
-----------------------------------------------------------

Ship it!


The only thing that we should change is the "patch" naming convention. We can revisit the other issues later. I don't want this blocking parallel work in this branch.

- Jonathan Hurley


On Sept. 9, 2015, 9:58 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:58 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review98202
-----------------------------------------------------------

Ship it!


Ship It!

- Jonathan Hurley


On Sept. 9, 2015, 11:08 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 11:08 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePack.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

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

(Updated Sept. 9, 2015, 3:08 p.m.)


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


Changes
-------

Renamed ConfigUpgradePatch to ConfigUpgradePack


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


Repository: ambari


Description
-------

The configs need to move out of the Upgrade Packs and into their own file.
This will make it easier to maintain, and clearer since there will not be any dups.

Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
  ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
  ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePack.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
  ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 

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


Testing
-------

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Ambari Views ...................................... SUCCESS [2.992s]
[INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
[INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 54:30.573s
[INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
[INFO] Final Memory: 60M/1072M


Thanks,

Dmitro Lisnichenko


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java, line 41
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065309#file1065309line41>
> >
> >     Can we change this to not include the term "patch". It will get confusing with the patch upgrade work coming soon. Maybe call it something like ConfigPack or ConfigurationPackage, etc.
> 
> Dmitro Lisnichenko wrote:
>     I was trying to avoid confusion with config versions. How about ConfigUpgradePackage?

I'm good with that. I just want to avoid the term "patch" in this feature.


> On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, line 347
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line347>
> >
> >     Any reason you are scoping these under a "changes" element? Seems redundant and unnecessary.
> 
> Dmitro Lisnichenko wrote:
>     I thought we may add few other sections later, so wanted to have a separate section for changes.
> 
> Dmitro Lisnichenko wrote:
>     It would be great to get the patch in asap to unblock other work. If we decide to remove <changes> section, may I address removing that in a next patch (adding config changes to SWU)?

OK. I'm fine with that. I just don't want to make this file overly verbose if it doesn't need to be.


> On Sept. 9, 2015, 10:02 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, lines 345-346
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line345>
> >
> >     Why are we placing the service/component in this configuration file? Seems like it's not needed. The IDs are referenced directly from the upgrade XML pack. Wouldn't this make it harder to lookup the IDs during upgrade?
> 
> Dmitro Lisnichenko wrote:
>     That is not required for ids to work, but it makes file much more obvious and manageable. So changes are grouped by owner and can be collapsed in IDE.
>     IDs are added to map and cached, and we don't deal with services/components in runtime when performing upgrade

I can see the value in that. But at the same time, it adds complexity to this file without any functional benefit (unless you can see a use case for querying for all configuration definitions by service/component).

To unblock, I'm fine leaving it for now. Please open a Jira to revisit this and the below <changes> element to see if they should be removed.


- Jonathan


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


On Sept. 9, 2015, 9:58 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:58 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Sept. 9, 2015, 2:02 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, line 347
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line347>
> >
> >     Any reason you are scoping these under a "changes" element? Seems redundant and unnecessary.
> 
> Dmitro Lisnichenko wrote:
>     I thought we may add few other sections later, so wanted to have a separate section for changes.

It would be great to get the patch in asap to unblock other work. If we decide to remove <changes> section, may I address removing that in a next patch (adding config changes to SWU)?


- Dmitro


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


On Sept. 9, 2015, 1:58 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 1:58 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Sept. 9, 2015, 2:02 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java, line 41
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065309#file1065309line41>
> >
> >     Can we change this to not include the term "patch". It will get confusing with the patch upgrade work coming soon. Maybe call it something like ConfigPack or ConfigurationPackage, etc.

I was trying to avoid confusion with config versions. How about ConfigUpgradePackage?


> On Sept. 9, 2015, 2:02 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, lines 345-346
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line345>
> >
> >     Why are we placing the service/component in this configuration file? Seems like it's not needed. The IDs are referenced directly from the upgrade XML pack. Wouldn't this make it harder to lookup the IDs during upgrade?

That is not required for ids to work, but it makes file much more obvious and manageable. So changes are grouped by owner and can be collapsed in IDE.
IDs are added to map and cached, and we don't deal with services/components in runtime when performing upgrade


> On Sept. 9, 2015, 2:02 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml, line 347
> > <https://reviews.apache.org/r/37682/diff/6/?file=1065315#file1065315line347>
> >
> >     Any reason you are scoping these under a "changes" element? Seems redundant and unnecessary.

I thought we may add few other sections later, so wanted to have a separate section for changes.


- Dmitro


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


On Sept. 9, 2015, 1:58 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 1:58 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review98176
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java (line 41)
<https://reviews.apache.org/r/37682/#comment154464>

    Can we change this to not include the term "patch". It will get confusing with the patch upgrade work coming soon. Maybe call it something like ConfigPack or ConfigurationPackage, etc.



ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml (lines 23 - 24)
<https://reviews.apache.org/r/37682/#comment154468>

    Why are we placing the service/component in this configuration file? Seems like it's not needed. The IDs are referenced directly from the upgrade XML pack. Wouldn't this make it harder to lookup the IDs during upgrade?



ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml (line 25)
<https://reviews.apache.org/r/37682/#comment154467>

    Any reason you are scoping these under a "changes" element? Seems redundant and unnecessary.


- Jonathan Hurley


On Sept. 9, 2015, 9:58 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 9:58 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Views ...................................... SUCCESS [2.992s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
> [INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 54:30.573s
> [INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
> [INFO] Final Memory: 60M/1072M
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

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

(Updated Sept. 9, 2015, 1:58 p.m.)


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


Changes
-------

Fixed comments, refactored all configure tasks in 2.2->2.2+, 2.2->2.3, 2.3->2.3+ upgrade packs to a new style. Going to commit patch if it looks good.

Created jiras for adding config change for SWU, and for fixing ignored unit tests after implementing cross-stack upgrade


Summary (updated)
-----------------

Stop-and-Start Upgrade: Move Configs out of Upgrade Pack


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


Repository: ambari


Description
-------

The configs need to move out of the Upgrade Packs and into their own file.
This will make it easier to maintain, and clearer since there will not be any dups.

Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
  ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
  ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/config-upgrade.xml PRE-CREATION 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 74eb499 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 044c43a 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
  ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 

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


Testing (updated)
-------

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Ambari Views ...................................... SUCCESS [2.992s]
[INFO] Ambari Metrics Common ............................. SUCCESS [1.675s]
[INFO] Ambari Server ..................................... SUCCESS [54:25.287s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 54:30.573s
[INFO] Finished at: Tue Sep 08 18:53:06 EEST 2015
[INFO] Final Memory: 60M/1072M


Thanks,

Dmitro Lisnichenko


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

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



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java (line 496)
<https://reviews.apache.org/r/37682/#comment154158>

    Without this change, RU can not be started at all


- Dmitro Lisnichenko


On Sept. 8, 2015, 3:37 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2015, 3:37 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

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

(Updated Sept. 8, 2015, 3:37 p.m.)


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


Changes
-------

Published a new patch. 
I've tested RU 2.2->2.3 for few services on live cluster, and config changes seem to work properly.
I've refactored all configure tasks for 2.2->2.3 to a new format 

My current plan is to:
- Move config changes for RU and SWU for 2.2->2.2+ to a separate file
- Commit the patch and close current jira 
- Open a separate jira to fix unit tests. As of now, unit tests are commented out/ignored. 
- Implement config upgrade for cross-stack upgrade
- Commit the patch to unblock cross-stack upgrade development and avoid further merges
- Complete fix unit tests jira


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


Repository: ambari


Description
-------

The configs need to move out of the Upgrade Packs and into their own file.
This will make it easier to maintain, and clearer since there will not be any dups.

Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java dddec73 
  ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
  ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java aa8e17b 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java db947ca 
  ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
  ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 87301e5 
  ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java ecefe6e 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/ConfigUpgradePatch.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java 9d89b7a 
  ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilder.java c9c6b8c 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml 7c1a1f9 
  ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java e702e0a 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java 2eee2df 
  ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java fc731d9 

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


Testing
-------

just published preview of changes


Thanks,

Dmitro Lisnichenko


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On Aug. 27, 2015, 1:40 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml, line 467
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056036#file1056036line467>
> >
> >     But what about cases where there a configuration done in the <order> section?
> 
> Dmitro Lisnichenko wrote:
>     How about implementing a virtual "apply all config changes for service" task type to handle this use case? When executing cross-stack upgrade (like 2.2->2.4), we are not going to parse and execute intermediate upgrade packs, but only apply config changes for installed services. So, when executing upgrade pack for target stack, "apply all config changes for service" task would apply all config changes for service defined at `upgrade-config-changes-*.xml` files for intermediate stacks. 
>     Does this approach sound good?

But we can still parse the configurations by an ID, right? The 2.2 to 2.4 upgrade pack won't parse intermediate packs, but it can still have configurations by ID from 2.2, 2.3, and 2.4. If you used a special task to "apply all configs", how are you going to determine which configs apply? By looking through all of the upgrade packs for all of the configs they reference? What about if there is an order issue where 2.2 sets a value to X, 2.3 checks value for X and sets to Y. In this case, it would never be set.


- Jonathan


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


On Aug. 27, 2015, 11:08 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 11:08 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Aug. 27, 2015, 5:40 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml, line 467
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056036#file1056036line467>
> >
> >     But what about cases where there a configuration done in the <order> section?
> 
> Dmitro Lisnichenko wrote:
>     How about implementing a virtual "apply all config changes for service" task type to handle this use case? When executing cross-stack upgrade (like 2.2->2.4), we are not going to parse and execute intermediate upgrade packs, but only apply config changes for installed services. So, when executing upgrade pack for target stack, "apply all config changes for service" task would apply all config changes for service defined at `upgrade-config-changes-*.xml` files for intermediate stacks. 
>     Does this approach sound good?
> 
> Jonathan Hurley wrote:
>     But we can still parse the configurations by an ID, right? The 2.2 to 2.4 upgrade pack won't parse intermediate packs, but it can still have configurations by ID from 2.2, 2.3, and 2.4. If you used a special task to "apply all configs", how are you going to determine which configs apply? By looking through all of the upgrade packs for all of the configs they reference? What about if there is an order issue where 2.2 sets a value to X, 2.3 checks value for X and sets to Y. In this case, it would never be set.

Our current vision (as discussed with Alejandro in "Re: Move out configs from upgrade pack - few discussion points" discussion) is that all config changes are referenced by ID. IDs share the same namespace of all stacks. And upgrade pack for 2.2->2.4 stack has to explicitly reference all config changes to be applied. Probably I'll implement a restriction that upgrade pack for 2.2->2.4 can only reference config changes for 2.3 and 2.4 stacks.


- Dmitro


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


On Aug. 27, 2015, 3:08 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 3:08 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Dmitro Lisnichenko <dl...@hortonworks.com>.

> On Aug. 27, 2015, 5:40 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java, lines 244-252
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056032#file1056032line244>
> >
> >     I'm still not a fan of the ordering being used as the main way of orchestrating the upgrades. What if there is a particular order the configurations need to be executed in? What if some happen in the <order> element and some in <processing>. Should we be using IDs instead and have an explicit link between upgrade pack and config pack.

We decided to move all config tasks from upgrade pack to a separate file. When executing cross-stack upgrade (like 2.2->2.4), we are not going to parse and execute intermediate upgrade packs, but only apply config changes from `upgrade-config-changes-*.xml` files for installed services. Service ordering for config application is going to be borrowed from target upgrade pack. So ID/linking approach may be not too flexible when upgrading over few stacks (we would have to be careful with config change coherense in different upgrade packs for the same stack). So to handle such cases I'd vote for using per-service config change priority (float-point value). Adding per-service config change priority (valid in scope of a single stack) should allow us to apply config changes in arbitrary order when executing changes. Let's discuss this solution on sync today if needed.


> On Aug. 27, 2015, 5:40 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml, line 467
> > <https://reviews.apache.org/r/37682/diff/5/?file=1056036#file1056036line467>
> >
> >     But what about cases where there a configuration done in the <order> section?

How about implementing a virtual "apply all config changes for service" task type to handle this use case? When executing cross-stack upgrade (like 2.2->2.4), we are not going to parse and execute intermediate upgrade packs, but only apply config changes for installed services. So, when executing upgrade pack for target stack, "apply all config changes for service" task would apply all config changes for service defined at `upgrade-config-changes-*.xml` files for intermediate stacks. 
Does this approach sound good?


- Dmitro


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


On Aug. 27, 2015, 3:08 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 3:08 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>


Re: Review Request 37682: [Preview] Stop-and-Start Upgrade: Move Configs out of Upgrade Pack

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37682/#review96707
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java (lines 244 - 252)
<https://reviews.apache.org/r/37682/#comment152347>

    I'm still not a fan of the ordering being used as the main way of orchestrating the upgrades. What if there is a particular order the configurations need to be executed in? What if some happen in the <order> element and some in <processing>. Should we be using IDs instead and have an explicit link between upgrade pack and config pack.



ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml (line 467)
<https://reviews.apache.org/r/37682/#comment152348>

    But what about cases where there a configuration done in the <order> section?


- Jonathan Hurley


On Aug. 27, 2015, 11:08 a.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37682/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 11:08 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan Hurley, and Nate Cole.
> 
> 
> Bugs: AMBARI-12700
>     https://issues.apache.org/jira/browse/AMBARI-12700
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The configs need to move out of the Upgrade Packs and into their own file.
> This will make it easier to maintain, and clearer since there will not be any dups.
> 
> Since it is going to be a massive change, it would be great to get early feedback. Code is not complete (still full of TODOs and does not even build)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/AmbariMetaInfo.java 4afa9b0 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java fa743be 
>   ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java c717582 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDefinitionDirectory.java 8f81b5a 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java 89c10c6 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 4b88aff 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 2aa89cc 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java 3e25d01 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradeConfigChangesDescriptor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java 8361ea6 
>   ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.2.xml 9b7848f 
>   ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java 93e29b5 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java f7898ee 
>   ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java a73775f 
>   ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade-config-changes-2.2.xml PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37682/diff/
> 
> 
> Testing
> -------
> 
> just published preview of changes
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>