ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Hurley" <jhur...@hortonworks.com>
Subject Re: Review Request 37682: Stop-and-Start Upgrade: Move Configs out of Upgrade Pack
Date Wed, 09 Sep 2015 14:41:14 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message