> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 844 > > <https://reviews.apache.org/r/45169/diff/2/?file=1382543#file1382543line844> > > > > The after tag is overloaded. Meaning it could mean insert this service > > after another service in some place and in some places it could mean insert > > this group after another group. I am wondering if there would be a case > > where we would need a combination (example: Create a new group that is not > > in the stack upgrade pack and add 2 new services to this group in a > > specific order). > > > > Instead of leaving the <after> tag to interpretation, why not be > > explicit as <add-group-after>, <add-services-after> etc. That way we can > > support combinations > > Jayush Luniya wrote: > Example: > > <group name="MY_MASTERS" title="My Masters"> > <add-group-after>CORE_MASTERS</add-group-after> > <service name="MY_SERVICE_1"> > <component>MY_MASTER_1</component> > </service> > </group> > > > <group name="MY_MASTERS" title="My Masters"> > <add-group-after>CORE_MASTERS</add-group-after> > <add-services-after>MY_SERVICE_1</add-services-after> > <service name="MY_SERVICE_2"> > <component>MY_MASTER_2</component> > </service> > </group> > > Alterative > > <group name="MY_MASTERS" title="My Masters"> > <after>CORE_MASTERS</after> > <service name="MY_SERVICE_1"> > <component>MY_MASTER_1</component> > </service> > </group> > > > <group name="MY_MASTERS" title="My Masters"> > <after>CORE_MASTERS</after> > <service name="MY_SERVICE_2"> > <after>MY_SERVICE_1</after> > <component>MY_MASTER_2</component> > </service> > </group> > > Tim Thorpe wrote: > With custom services they should just be able to create two new groups > rather than try to combine them into one. If there really is a need for the > a new group that spans multiple services, then it is probably a candidate for > inclusion in the main upgrade xml. You can even include an empty group in > the main upgrade xml like: > > <group name="MY_MASTERS" title="My Masters" /> > > Jayush Luniya wrote: > RE: If there really is a need for the a new group that spans multiple > services, then it is probably a candidate for inclusion in the main upgrade > xml. > Well these services are add-on services meaning say not part of HDP stack > itself (example: HAWQ and PFX add-on services on HDP stack). So putting them > in stack upgrade pack wouldnt be right. > > Yes these can be in 2 separate groups, but that would be because of this > restriction. > > Besides its also confusing when we overload the same property element. > For example the group name in upgrade pack could be the service name itself > like below, so <after>STORM</after> could mean after service STORM or after > group STORM :) > > > https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml#L356-L364 > <group name="STORM" title="Storm"> > <skippable>true</skippable> > <service name="STORM"> > <component>NIMBUS</component> > <component>SUPERVISOR</component> > <component>STORM_UI_SERVER</component> > <component>DRPC_SERVER</component> > </service> > </group>
Currently, I'm looping through all the groups from the main upgrade xml. I add those to a <String, List<Grouping>> map. This means the group from the main upgrade xml (if there was one) will be the first group in the list. This is important because I know those groups are already internally ordered. By that I mean there services/priorities/stages do NOT need to be shifted in the overall order (all insertions into that order are determined by the subsequent groups in the list). Currently when I process the service upgade xml files, for each group if the group name already exists in the map then this means the group existed in the main upgrade xml and the entries (services/priorities/stages) will be inserted into the original entry order based on the <after> tag. If the group name doesn't exist, it is added with its group as the first group in the list. In this case I never expect another group with the same name to be added to that list. The missing piece is with this change I need to reorder the List<Grouping> in the map to find a group that doesn't have an <add-after-group-entry> and if all groups do have that tag, then I guess I have an error situation. Adding explicit tags for <add-after-group> and <add-after-group-entry> would make it easier to understand. I would prefer to limit it to two tags <add-after-group> and <add-after-group-entry> rather than split the <add-after-group-entry> into separate tags for services/priorities/stages. - Tim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133556 ----------------------------------------------------------- On May 18, 2016, 6:26 p.m., Tim Thorpe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45169/ > ----------------------------------------------------------- > > (Updated May 18, 2016, 6:26 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush > Luniya, and Nate Cole. > > > Bugs: AMBARI-15388 > https://issues.apache.org/jira/browse/AMBARI-15388 > > > Repository: ambari > > > Description > ------- > > Currently the upgrade is defined as a series of xml files specific to the > current stack version and the target stack version. Each upgrade xml defines > the overall sequence of the upgrade and what needs to be done for each > service. It would both easier to maintain and easier to add new services, if > the services themselves could specify what should be done during their > upgrade. > > There are two ways to make these changes, the alternate approach would be to > only make the java changes and not split the upgrade xml files. This would > still allow new services to add themselves into the upgrade. The benefit of > this is that for the stack services you only have one upgrade xml file. The > problem with that is it is easier for a particular service to have > unintentional changes between upgrade xml files. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java > 7f7a49e > > ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java > 8a7b42b > > ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java > f781574 > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java > 13d5047 > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java > 5a18b3f > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java > 88f6e19 > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java > 43cefb9 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java > b860731 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ClusterGrouping.java > 3325469 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > 67d7fdb > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java > 5cda422 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml > 6b74af0 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml > 9fb2bba > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml > 1e040e6 > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml > e3bc7a3 > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml > 6e27da6 > ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml > d755516 > > ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerMiscTest.java > dda1e7a > > ambari-server/src/test/java/org/apache/ambari/server/state/stack/UpgradePackTest.java > 15be8b4 > > ambari-server/src/test/resources/stacks/HDP/2.0.5/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_15388.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/metainfo.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/hdp.json > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/repoinfo.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/repos/version-2.2.0.4-123.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/role_command_order.json > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/metainfo.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/services/HDFS/upgrades/HDP/2.2.0/upgrade_test_15388.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/config-upgrade.xml > PRE-CREATION > > ambari-server/src/test/resources/stacks_with_upgrade_cycle/HDP/2.2.0/upgrades/upgrade_test_15388.xml > PRE-CREATION > > Diff: https://reviews.apache.org/r/45169/diff/ > > > Testing > ------- > > Manual testing so far. I have the code read the upgrade xml and all of its > service specific xml files, built the upgrade pack and then write the full > upgrade xml to disk and then compare the results to the original upgrade xml. > > This review is mostly for the design doc which is attached to the JIRA. Not > sure how to create a review board with a design doc instead of a patch file. > > > Thanks, > > Tim Thorpe > >
