> On Jan. 4, 2016, 3:38 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java, > > lines 435-439 > > <https://reviews.apache.org/r/41774/diff/4/?file=1180185#file1180185line435> > > > > This seems dangerous. This had to have been added because some custom > > commands require the cluster topology to work correctly (not just for > > installing packages?). Maybe check if the object exists and is zero-size > > and add it if it's not there?
we don't need this info for installing packages. Myroslav says that execCmd.setClusterHostInfo() is obsolete and should not be used, since we are always setting/using clusterHostInfo on stage level. So it is not required at all, and slows down every addExecutionCommandsToStage() call I checked org.apache.ambari.server.controller.AmbariActionExecutionHelper#addExecutionCommandsToStage() calls, and at all places we are setting per-stage clusterHostInfo json when creating a stage - Dmitro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41774/#review112569 ----------------------------------------------------------- On Dec. 31, 2015, 5:47 p.m., Dmitro Lisnichenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41774/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2015, 5:47 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Dmytro Grinenko, Jonathan > Hurley, Jayush Luniya, Nate Cole, and Yusaku Sako. > > > Bugs: AMBARI-14520 > https://issues.apache.org/jira/browse/AMBARI-14520 > > > Repository: ambari > > > Description > ------- > > On 900 host cluster the call to install packages took like 3 minutes 20 > seconds which is kind of long. > {noformat} > POST http://perf-a-1:8080/api/v1/clusters/c1/stack_versions > > {"ClusterStackVersions":{"stack":"HDP","version":"2.2","repository_version":"2.2.9.0-3393"}}: > {noformat} > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariActionExecutionHelper.java > 6e7e06b > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterStackVersionResourceProvider.java > b6ac291 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostStackVersionResourceProvider.java > 43a4423 > > ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java > 3358e8c > > ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java > c56d7ce > > Diff: https://reviews.apache.org/r/41774/diff/ > > > Testing > ------- > > mvn clean test > > Current request execution time: 0m22.678s > > With current patch, most time (about 40%) is consumed by > findConfigurationTagsWithOverrides(cluster, hostname), but it seems to be > inevitable even with batching addExecutionCommandsToStage() calls into a > single invocation. I've tried inserting logging statements to verify that > number of invocations equals to number of hosts. > Rest of time is mostly consumed by > org.apache.ambari.server.actionmanager.Stage.addHostRoleExecutionCommand() > (20% of time) and by N invocations of ClustersImpl.getHostsForCluster() (19% > of time). The latter calls can not be cached in method, so I can only rewrite > method signature > AmbariActionExecutionHelper.addExecutionCommandsToStage(ActionExecutionContext, > Stage) and cache it in ClusterStackVersionResourceProvider. Seems to be a > bad deal considering 1-3 seconds (on large clusters ) of performance boost > for a rare call vs readability/maintainability of code. > > > Thanks, > > Dmitro Lisnichenko > >
