> On April 12, 2017, 10:12 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java > > Line 469 (original), 469-474 (patched) > > <https://reviews.apache.org/r/58367/diff/1/?file=1689164#file1689164line469> > > > > Guard against a 0 here with something like Max(getTimeout(), 1)?
Interesting point - good one, I'll guard against that. But if some clown is actually using 0 or negative they should get what's coming :) > On April 12, 2017, 10:12 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > > Lines 130-131 (original), 130-132 (patched) > > <https://reviews.apache.org/r/58367/diff/1/?file=1689166#file1689166line130> > > > > You removed the TaskWrapper though, right? I'm assuming b/c it wasn't > > used - so does this method really generate a task wrapper? I didn't remove TaskWrapper at this time. It's still needed as it defines the Task that should run, and on what hosts. The fix for later is that TaskWrapper holds List<Task>, which is wrong. I only made this clarifying comment for when that fix is made. > On April 12, 2017, 10:12 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml > > Line 551 (original), 551 (patched) > > <https://reviews.apache.org/r/58367/diff/1/?file=1689173#file1689173line551> > > > > Just to confirm; this can currently be set in ambari.properties, right? > > That way nobody has to go changing python or XML files ... > > > > What about specifying this in the upgrade request? I know the UI > > doesn't have that option yet, but maybe we should open Jiras for it. That > > way a restart isn't required. This means the UI has to A) understand what properties can be passed, implying we have to provide those names somehow and B) build the screen for this, which is undefined for 2.5.1. We can revisit for 3.0 if it's really needed. Also means we would have to find/define every service that requires a special timeout; so far only NN has been found to be an issue. > On April 12, 2017, 10:12 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml > > Line 582 (original), 582 (patched) > > <https://reviews.apache.org/r/58367/diff/1/?file=1689174#file1689174line582> > > > > I'm fine this the granularity of this since it gets us to where we want > > to be. Oringinally, I thought we'd do something more generic like a timeout > > for master start - but I guess NN restart could take a much longer time > > than Nimbus. I thought about that, but since we don't have all the places in python where to expect this value, it becomes a bit misleading about whether we have implemented it or not. The NN case is simple because we know where the holdup is. For other daemons we can just let the server manage the timeout when @retry isn't used. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58367/#review171733 ----------------------------------------------------------- On April 11, 2017, 3:21 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58367/ > ----------------------------------------------------------- > > (Updated April 11, 2017, 3:21 p.m.) > > > Review request for Ambari, Alejandro Fernandez and Jonathan Hurley. > > > Bugs: AMBARI-20736 > https://issues.apache.org/jira/browse/AMBARI-20736 > > > Repository: ambari > > > Description > ------- > > Add the ability to set overrides for certain tasks in Upgrade Packs. This > was done via ambari.properties for now. Essentially, tasks may be defined > with a timeout-config option that will perform a lookup and apply it to the > ActionExecutionContext instance. So far only one task was identified that > requires this, and that's NN restarts. As we discover other time consuming > tasks we can add them, or alternatively show customers how to do this for > their own upgrades. > > I did uncover an issue where we associate multiple Task objects to a > TaskWrapper, which is wrong (should be one-to-one). In addition, we aren't > setup currently to allow multiple tasks to run concurrently on one host. We > DO allow multiple tasks across hosts to work in parallel, which is fine. > These issues will be addressed in a future patch. > > > Diffs > ----- > > > ambari-common/src/main/python/resource_management/libraries/functions/decorator.py > 55cf335aa0 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelper.java > d5018f55b2 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > 709ca9360d > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java > cd17a7080a > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java > aac89358da > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Task.java > 5c43c2befd > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapper.java > 11e27cfb9f > > ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapperBuilder.java > a75fe00bd4 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/hdfs_namenode.py > 04897927f6 > > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/scripts/params_linux.py > f0566d78e4 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml > 1340b22711 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml > 40afc4f356 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml > e0882d8350 > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.6.xml > 0f4efdcd0d > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml > d5e9a5b095 > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml > 350395c714 > ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.6.xml > 9ac3d52fe8 > ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml > 04a06e8447 > ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.6.xml > 879fe0f9bd > ambari-server/src/main/resources/stacks/HDP/2.6/upgrades/upgrade-2.6.xml > fd72e4d69e > ambari-server/src/main/resources/upgrade-pack.xsd 1f11aa1bd4 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > 999b7a7612 > ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml > 8d506bfd4e > > > Diff: https://reviews.apache.org/r/58367/diff/1/ > > > Testing > ------- > > Tests run: 4980, Failures: 0, Errors: 0, Skipped: 39 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 24:22 min > [INFO] Finished at: 2017-04-11T15:05:10-04:00 > [INFO] Final Memory: 47M/216M > [INFO] > ------------------------------------------------------------------------ > > > ---------------------------------------------------------------------- > Total run:1192 > Total errors:0 > Total failures:0 > OK > > > Thanks, > > Nate Cole > >
