> On Aug. 3, 2016, 2:36 a.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. > > Renan DelValle wrote: > 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.) > > Joshua Cohen wrote: > I'm ok with looking into e2e tests in a follow up patch. Would you mind > filing a ticket to track that? > > As far as changing the executor name goes, I'm neutral on that. I'm not > sure there's a huge upside, and there are potential downsides to it (e.g. > would it cause executor config to change for an otherwise no-op update)? > > Renan DelValle wrote: > No problem. > > Yeah, I'm neutral on this as well. The only benefit I see is being able > to test this feature, but I'm not sure about the ripple effects it may have. > > The alternative is using a custom client (which we're in the process of > preparing for release). The downsides here are obvious but it may be the > least painful option. We can continue discussion here: > https://issues.apache.org/jira/browse/AURORA-1744 > > Thanks again for looking over the code, very much appreciated. > > Renan DelValle wrote: > Whoops sorry, crossed some cable here. > Yeah, no huge upsides on changing the name on the Thrift API just wanted > to bring it up in case anyone felt strongly about it. > > Will look into being able to set executor name from Pystachio to test > this feature.
Would you mind rebasing this and posting an updated diff. I just tried to merge and I'm getting conflicts with my recently submitted isolation changes. - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50480/#review144579 ----------------------------------------------------------- On Aug. 2, 2016, 11: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, 11: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 > >
