> On Dec. 28, 2016, 9:36 a.m., Jonathan Hurley wrote: > > Although I do not see anything wrong with this implementation, I'm curious > > why we simply can't use a single lock for this. Why do we need specific > > locks per component? > > > > Shouldn't it be enough to have a single lock which is used during the > > installation phase as well as the conf-select symlinking phase? The goal > > here is to just ensure they don't run in parallel. > > Sebastian Toader wrote: > In order to improve cluster creation time we need to run in parallel as > much as possible and synchronize only those bits we know can't run in > parallel. During installation of two components we we should run in parallel > the bits that are independent. > > Jonathan Hurley wrote: > I think that's my point - none of the install commands should ever run at > the same time as the conf-select work. conf-select should take less than a > second to run and re-link. I don't think it's going to break the bank here. I > think it's much safer to ensure that they are 100% mutually exclusive. > > Attila Doroszlai wrote: > This change introduces lock usage for configure() only for TEZ. With > this setup, there is no noticeable difference between single lock and > multiple locks. If we were to lock more/all configure() calls, the > difference would be more significant. > > Jonathan Hurley wrote: > Would it be that much more significant? You're locking on running yum > installs in parallel with conf-select post-install work, right? The > conf-select work is extremely fast, especially compared to the time it takes > to actually do an install. Therefore, doesn't it make sense to use a less > complicated implementation with a single lock? > > Attila Doroszlai wrote: > No, yum installs are already done by that time. Only configure() (which > also copies and links configs) and conf-select run with mutex.
Hmmm, looking at this patch again, you're only using `get_config_lock` in two places: - After Tez's package install before calling `configure` - On AFTER-INSTALL while iterating every package when linking configs This still poses a problem since you could have a parallel install of another component while AFTER-INSTALL iterates packages and tries to symlink that component. That's why I'm saying that the fine grain locks here aren't really solving the problem. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55016/#review160228 ----------------------------------------------------------- On Jan. 2, 2017, 10:47 a.m., Attila Doroszlai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55016/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2017, 10:47 a.m.) > > > Review request for Ambari, Andrew Onischuk, Balázs Bence Sári, Jonathan > Hurley, Laszlo Puskas, and Sebastian Toader. > > > Bugs: AMBARI-19293 > https://issues.apache.org/jira/browse/AMBARI-19293 > > > Repository: ambari > > > Description > ------- > > * Perform TEZ `configure` step with lock (will add other services later). > * Use service-specific locks in after-install hook to decrease contention > > > Diffs > ----- > > > ambari-common/src/main/python/resource_management/libraries/functions/conf_select.py > ce00f0c6f69f255690f1ab9528bea6cee184a73e > > ambari-server/src/main/resources/common-services/TEZ/0.4.0.2.1/package/scripts/tez_client.py > 8018f0f960322aac196caf3915a0a944e7765d8e > > ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/after-INSTALL/scripts/params.py > ed34217757540278a8730d58797cc47536e58874 > > ambari-server/src/main/resources/stacks/HDP/2.0.6/hooks/after-INSTALL/scripts/shared_initialization.py > e9f22834b5479330a6f00ca69991cc690780b3c3 > ambari-server/src/test/python/stacks/2.2/common/test_conf_select.py > d445d741ba4225a33e2a763eed6dfcf8394dceb6 > > Diff: https://reviews.apache.org/r/55016/diff/ > > > Testing > ------- > > Manual test: > > * install cluster via blueprint with parallel execution disabled (this is the > default) > * install cluster via blueprint with parallel execution enabled (needs > packages pre-installed and users pre-created) > > Unit tests: > > ``` > $ mvn -am -pl ambari-agent,ambari-server -DskipSurefireTests clean test > ... > [INFO] --- exec-maven-plugin:1.2.1:exec (python-test) @ ambari-server --- > ... > Total run:1160 > Total errors:0 > Total failures:0 > OK > ... > [INFO] --- exec-maven-plugin:1.2.1:exec (python-test) @ ambari-agent --- > ... > Ran 451 tests in 15.344s > > OK > ... > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Main ....................................... SUCCESS [6.650s] > [INFO] Apache Ambari Project POM ......................... SUCCESS [0.027s] > [INFO] Ambari Views ...................................... SUCCESS [2.355s] > [INFO] utility ........................................... SUCCESS [1.182s] > [INFO] ambari-metrics .................................... SUCCESS [0.315s] > [INFO] Ambari Metrics Common ............................. SUCCESS [0.730s] > [INFO] Ambari Server ..................................... SUCCESS [1:38.241s] > [INFO] Ambari Agent ...................................... SUCCESS [17.247s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > ``` > > > Thanks, > > Attila Doroszlai > >
