aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Renan DelValle <rdelv...@binghamton.edu>
Subject Re: Review Request 50480: Multiple executor support in Scheduler
Date Wed, 03 Aug 2016 19:35:57 GMT


> 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.
> 
> 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)?

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


-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message