> On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: > > src/test/python/apache/aurora/client/api/test_updater.py, line 126 > > <https://reviews.apache.org/r/21440/diff/1/?file=581731#file581731line126> > > > > Doesn't this mean that the test will be running with just one thread? > > That will serialize everything, which will mask a lot of potential error > > cases.
Correct. All unit tests are using a single worker thread to ensure verifiability. This covers the majority of the updater mechanics with real concurrency covered by e2e tests and runtime verification. Having multiple worker threads would require a huge test-rewriting effort and would result in a potential loss of coverage and flakiness. I would rather not do that and defer concurrency testing to runtime given that it's impossible to exhaustively cover all threading edge cases with mock unit tests. > On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 357 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line357> > > > > What is this supposed to return? Fixed here and in other places. > On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 325 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line325> > > > > It took me several readthroughs to figure out what this was doing. Is > > there maybe a clearer way of writing this? All other ideas I can come up with result in more code. Any ideas? > On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/updater.py, line 237 > > <https://reviews.apache.org/r/21440/diff/1/?file=581729#file581729line237> > > > > Why? See reply to Brian. > On May 14, 2014, 7:20 p.m., Mark Chu-Carroll wrote: > > src/main/python/apache/aurora/client/api/instance_watcher.py, line 127 > > <https://reviews.apache.org/r/21440/diff/1/?file=581726#file581726line127> > > > > Doc comment please. It's not clear reading this just what "terminating" > > or "terminate" means. It would be a huge help to just put a couple of words > > here. Done. - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21440/#review43004 ----------------------------------------------------------- On May 14, 2014, 6:49 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21440/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 6:49 p.m.) > > > Review request for Aurora, Mark Chu-Carroll and Brian Wickman. > > > Bugs: AURORA-350 > https://issues.apache.org/jira/browse/AURORA-350 > > > Repository: aurora > > > Description > ------- > > The updater now spawns upto batch_size threads to process one instance per > thread. > > All mutating calls are multiplexed by the SchedulerMux to do batch > kill/add/restart calls. This is the first step towards a fully multiplexed > SchedulerProxy and is intended to mitigate LDAP/scheduler load. > > > Diffs > ----- > > src/main/python/apache/aurora/client/api/instance_watcher.py > 8992b93d25ce71b5784bad24429fd32356f64763 > src/main/python/apache/aurora/client/api/job_monitor.py > a27f6fbbf38e981f259aea35c9bf979d2d9b1b87 > src/main/python/apache/aurora/client/api/scheduler_client.py > ff5d2bc52af75ff2d2eee2d9b72c8d9a0df12581 > src/main/python/apache/aurora/client/api/updater.py > e8692ea4fdf924fb1ea06803d5e7321340754442 > src/main/python/apache/aurora/client/api/updater_util.py > 5ea96cd2797c4957d05168fb7ba9f950fd82bbbf > src/test/python/apache/aurora/client/api/test_updater.py > 03a14186646daede689ac77e2ab7144aa5a7fa14 > src/test/python/apache/aurora/client/cli/test_update.py > a357c44c87065474bce47fa403ddd72625da072d > src/test/python/apache/aurora/client/cli/util.py > 9d51a2e1fe961d4b522ac5b70f0737130326abe9 > src/test/python/apache/aurora/client/commands/test_update.py > 0b1e3498311d946631452e45ea2c7edb100dc43c > src/test/python/apache/aurora/client/commands/util.py > fc9e2f17537e1e731381b825ac4b5646e5130654 > src/test/python/apache/aurora/client/fake_scheduler_proxy.py > 6a1f78b97d67adab0cf00acc31f20c5d6197a508 > src/test/sh/org/apache/aurora/e2e/flask/flask_example.aurora > ad5aafec1f796baef766ed12b2a597f26997d5b3 > src/test/sh/org/apache/aurora/e2e/flask/flask_example_updated.aurora > 6d54463c349699946c2063385b1bd38a0f8c9e58 > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > c8fa07a5426c6c7748412062c26996b84637efb3 > src/test/sh/org/apache/aurora/e2e/test_end_to_end_v2.sh > 3c4165c8464b42f7f5dc4f94a079ff7c3086cdbc > > Diff: https://reviews.apache.org/r/21440/diff/ > > > Testing > ------- > > ./pants src/test/python:all > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Maxim Khutornenko > >
