> On Nov. 2, 2016, 11:10 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, > > lines 1880-1881 > > <https://reviews.apache.org/r/53373/diff/1/?file=1551380#file1551380line1880> > > > > Wow. Will this always be the correct order? Is our mystery parser > > making LinkedHashSets or HashSets?
Yeah, I thought this was going to be really bad since Eclipse showed it as a HashSet. But it turns out when I inspected the class instance it was actually a LinkedHashSet. > On Nov. 2, 2016, 11:10 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, > > lines 1367-1373 > > <https://reviews.apache.org/r/53373/diff/1/?file=1551380#file1551380line1367> > > > > Kind of a weird syntax anyway, can we just use the enhanced iterator of > > the entrySet? (I know this isn't your code, but while we're here...) OK, I'll update it. > On Nov. 2, 2016, 11:10 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, > > lines 2218-2222 > > <https://reviews.apache.org/r/53373/diff/1/?file=1551380#file1551380line2218> > > > > Could probably skip this and go right to the disjunction check I put this in in case there there are 100's of hosts and 1 is missing. It would prevent a massive message. But you know what, it's good to know which is missing, so I'll remove it. > On Nov. 2, 2016, 11:10 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java, > > lines 2238-2241 > > <https://reviews.apache.org/r/53373/diff/1/?file=1551380#file1551380line2238> > > > > Interesting concept. I like the idea, but this will only work if we > > are one-cluster-at-a-time. If we ever invoke an upgrade against two > > clusters to the same version and one of them is orchestrating while the > > other is started (very highly unlikely today) this won't work. I wouldn't > > be opposed to putting it on the context. Your call. Bah! I had thought that the UpgradePack was new objects every time. You're right that this would be problematic for concurrent upgrades. However, since we don't have that problem now (and we could get around it in the future by have a new UpgradPack instance for every upgrade), I think it's OK to leave it. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53373/#review154557 ----------------------------------------------------------- On Nov. 2, 2016, 8:11 a.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53373/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2016, 8:11 a.m.) > > > Review request for Ambari, Dmitro Lisnichenko and Nate Cole. > > > Bugs: AMBARI-18685 > https://issues.apache.org/jira/browse/AMBARI-18685 > > > Repository: ambari > > > Description > ------- > > Unit tests are in progress, but seeing as though this is a feature branch, I > didn't think I needed to wait to post the review. I mostly wanted feedback on > the approach before I invested time in the tests. > > Augment the API of creating an Upgrade with the following: > > ``` > POST api/v1/clusters/c1/upgrades > { > "Upgrade": { > "repository_version": "2.5.0.0-965", > "upgrade_type": "HOST_ORDERED", <---------- new value other than > ROLLING/NON_ROLLING > "skip_failures": "false", > "skip_prerequisite_checks": "false", > "skip_manual_verification": "false", > "host_order": [ <------ new > { > "hosts": [ "c6401.ambari.apache.org, "c6402.ambari.apache.org", > "c6403.ambari.apache.org" ], > "service_checks": ["ZOOKEEPER"] > }, > { > "hosts": [ "c6404.ambari.apache.org, "c6405.ambari.apache.org"], > "service_checks": ["ZOOKEEPER", "KAFKA"] > } > ] > } > } > ``` > > {{skip_failures}} must be {{false}} when the upgrade_type is HOST_ORDERED > {{skip_manual_verification}} must be omitted or {{false}} when upgrade_type > is HOST_ORDERED > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > d83aaa2 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/HostOrderGrouping.java > 9c75344 > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/HostOrderItem.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/53373/diff/ > > > Testing > ------- > > PENDING > > > Thanks, > > Jonathan Hurley > >
