> On Aug. 2, 2016, 7:36 p.m., Joshua Cohen wrote: > > I was about to push this change, but in the process of running e2e tests > > locally after applying your patch, it occurs to me that perhaps this is > > something we should have e2e coverage for. > > > > Do you think it would be feasible to add something to test_end_to_end.sh > > that verifies the multiple executor (and transitively the custom executor) > > support? If it would be a big change, I'd be ok with shipping it in a > > follow up review.
This is a far point and one I considered as well. It would be of great benefit to add a test but I think it might be a bigger change than it seems on the surface. One of the biggest pain points to getting this done is being able to submit tasks with a configurable ExecutorConfig.name using pystachio. For now the best we have technical support for is being able to load multiple executors sucessfully which is one of the test for ExecutorSettingsLoader. That said, I'll be looking into it and would gladly provide a follow up patch. On a slight side note, since we'll be using ExecutorConfig.name to identify the executor to use, the current default for thermos is "aurora_executor" (as set on the thrift api). If we want to change this to "thermos", now's the time to do it. (From what I've seen this constant was not used anywhere in the Server side code until this patch.) - Renan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144579 ----------------------------------------------------------- On Aug. 2, 2016, 4:38 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50480/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2016, 4:38 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > Adds support for using multiple executors in a single scheduler. > > Added warnings for pre-emption performance degradation when increasing > executor overhead. > > Made executor config file require a JSON array. > > Modified the way in which Thermos and any custom executors are loaded at > startup. > > Thermos now always exists regardless of any other executors being used. > > Jobs sent where no executor configuration is found for them are rejected. > > Task prefix is now set from json file. > > Fixed non revokable resources not being filtered out from overhead. > > > Diffs > ----- > > RELEASE-NOTES.md 19d7f7e34652d170a7d8eb41a9c6ffdcc67324eb > docs/operations/configuration.md 0615c545f560597683f727c2425de4cc4e82c364 > src/main/java/org/apache/aurora/GuavaUtils.java > 8c2ab5720b808e5eae4c6b93fea8b787839b8dbe > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > c43e04aee0df8988ed3af9d463dd6747d6631e3b > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 93ed3600268f231b0e53ceb6b3674ff742d94407 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorConfig.java > 24329a49155b9e8f6d11dd1d09f6f5188d1e2ad9 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > 8b7f8dcf4d8e0372208e636b3f57fc159272ecc7 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java > e919d3f2e2f86c26c0448029b06277a3a41a6941 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java > b74edf46d35ac99164ec1bcf33edf36556baf9ed > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > f9b1c7cf30f93336fb850da09c1f2b7178cbdc17 > src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java > 1dfa97e659c2fca8308cb592f37d14328e4b42bc > > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java > ee6fe95eaa41c7952a974ebea069b3de6ed82994 > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java > 3b84dbcfde9ae686110409173d742b3fa86ac764 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 147327036a872c9ccd03e17daaaf8cb1df489843 > > src/test/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoaderTest.java > 7de7c46e6e1f478e25f7362d1d060b7f765dcb36 > > src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java > d34e12f6e0837fa43ea9b4e2a17db579e0ee10a2 > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > bb8a849717a6ca3328ad4638acff5cc44ddd6ac9 > > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java > 7eb1714d14581a6ab25e85d36a1f3e973380c536 > > src/test/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImplTest.java > fba427bd327e7f63b640c8b8753bfdeec3ee31e7 > src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java > a79b0f413e603374347f8ce5765fb6e71bc9a5b5 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > c0fe84371ff2f9d47721126a9c356180f7c166de > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-missing-field.json > 464617028b1e765a563a3e11f70d66089ede6866 > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-multiple-executor.json > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-single-executor.json > PRE-CREATION > > src/test/resources/org/apache/aurora/scheduler/configuration/executor/test-thermos-executor.json > 114eb4f6c3eaeca3ad976e4907a3ec32c2339a09 > > Diff: https://reviews.apache.org/r/50480/diff/ > > > Testing > ------- > > ./build-support/jenkins/build.sh > > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > Created and killed jobs running a custom executor using a custom client. > > Created and killed thermos jobs with the Aurora Client. > > > Thanks, > > Renan DelValle > >
