Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144693
---



Master (a071af3) is red with this patch.
  ./build-support/jenkins/build.sh

 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 ERROR] Could not quitquitquit runner: Cannot take control 
of a task in terminal state.
 E0803 22:51:49.279841 5688 thermos_task_runner.py:239] 
Could not quitquitquit runner: Cannot take control of a task in terminal state.
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  1 failed, 684 passed, 6 skipped, 1 warnings in 
283.16 seconds 
 
FAILURE


22:52:43 05:06   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 3, 2016, 10:01 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated Aug. 3, 2016, 10:01 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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/
---

(Updated Aug. 3, 2016, 3:01 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Fixing conflicts when merging most recent master.


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 (updated)
-

  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 
0b41dba3a1b3991b8ad9eb472c09e357659c8a22 
  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 
0be5e49b23c25ba1af28d848e0419253e0d502e5 
  
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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Joshua Cohen 


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle 


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

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.


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle 


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-03 Thread Renan DelValle 


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144579
---



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.

- Joshua Cohen


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144578
---


Ship it!




Ship It!

- Joshua Cohen


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144566
---



Master (b912e17) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/user/10021/tmpklLACQ', 
'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pants/setup/bootstrap-Linux-x86_64/1.1.0-rc7/bin/pants",
 line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  16 failed, 658 passed, 6 skipped, 1 warnings, 8 
error in 160.14 seconds 
 
FAILURE


23:54:05 03:10   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Renan DelValle 

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


Changes
---

Reverted change to how preemtable resources are filtered.


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 (updated)
-

  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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Maxim Khutornenko 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144549
---


Ship it!




LGTM (barring minor change in the last review cycle).

- Maxim Khutornenko


On Aug. 1, 2016, 9:54 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated Aug. 1, 2016, 9:54 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
> 
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-02 Thread Maxim Khutornenko 


> On Aug. 1, 2016, 6:28 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java,
> >  lines 117-121
> > 
> >
> > This will add CPU overhead of a revocable task, which does not actually 
> > address your earlier TODO. I suggest rewriting it the way I suggested in 
> > the previous round to only add non-revocable overhead of a revocable victim.
> 
> Renan DelValle wrote:
> Just for my own understanding, wouldn't the call to filter in the if 
> statement:
> ```java 
> bag = bag.filter(IS_MESOS_REVOCABLE.negate());
> ``` 
> get this done since overhead is part of the bag already?
> 
> Fixed, changed to what you suggested earlier.

You're correct, it would. No idea what I was thinking there. Please, revert to 
your last version as it was much better before my suggestion!


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144368
---


On Aug. 1, 2016, 9:54 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated Aug. 1, 2016, 9:54 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 
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144392
---


Ship it!




Master (b912e17) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 1, 2016, 9:54 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated Aug. 1, 2016, 9:54 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
> 
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Renan DelValle 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/
---

(Updated Aug. 1, 2016, 2:54 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Code style fixed. Changed code relating to how preemtable resources are 
calculated.


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 (updated)
-

  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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Renan DelValle 


> On Aug. 1, 2016, 11:28 a.m., Maxim Khutornenko wrote:
> > docs/operations/configuration.md, line 154
> > 
> >
> > drop trailing space

Fixed.


> On Aug. 1, 2016, 11:28 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java,
> >  lines 117-121
> > 
> >
> > This will add CPU overhead of a revocable task, which does not actually 
> > address your earlier TODO. I suggest rewriting it the way I suggested in 
> > the previous round to only add non-revocable overhead of a revocable victim.

Just for my own understanding, wouldn't the call to filter in the if statement:
```java 
bag = bag.filter(IS_MESOS_REVOCABLE.negate());
``` 
get this done since overhead is part of the bag already?

Fixed, changed to what you suggested earlier.


> On Aug. 1, 2016, 11:28 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  line 108
> > 
> >
> > Since nothing else references this field, feel free to inline it.

Good catch, thought I was referecing it elsewhere for some reason. Fixed.


- Renan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144368
---


On July 28, 2016, 3:35 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 28, 2016, 3:35 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 d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   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 
> 500fd435b4c72b25abd8df7eea6b3850edc96e99 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  7eb1714d14581a6ab25e85d36a1f3e973380c536 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImplTest.java
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-08-01 Thread Maxim Khutornenko 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144368
---




docs/operations/configuration.md (line 154)


drop trailing space



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 108)


Since nothing else references this field, feel free to inline it.



src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
(lines 117 - 121)


This will add CPU overhead of a revocable task, which does not actually 
address your earlier TODO. I suggest rewriting it the way I suggested in the 
previous round to only add non-revocable overhead of a revocable victim.


- Maxim Khutornenko


On July 28, 2016, 10:35 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 28, 2016, 10:35 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 d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   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 
> 500fd435b4c72b25abd8df7eea6b3850edc96e99 
>   
> 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 
>   
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review144025
---


Ship it!




Master (dde2c92) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 28, 2016, 10:35 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 28, 2016, 10:35 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 d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   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 
> 500fd435b4c72b25abd8df7eea6b3850edc96e99 
>   
> 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
> 
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/
---

(Updated July 28, 2016, 3:35 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

Fixed code formatting issues. Implemented feedback.

Task prefix is can now be set from the json config file.

Fixed non revokable resources not being filtered out from overhead.

@ReviewBot retry


Repository: aurora


Description (updated)
---

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 (updated)
-

  RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
  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 
cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
  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 
500fd435b4c72b25abd8df7eea6b3850edc96e99 
  
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 (updated)
---

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

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle 


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java, 
> > line 158
> > 
> >
> > Are we guaranteed that the executor config exists here? The previous 
> > branch is checking if executorConfig is set and that the config exists, but 
> > won't we fall into this branch if executorConfig is not set?
> 
> Renan DelValle wrote:
> Great catch! I forgot to put in some logic to handle the case where it's 
> a Docker task with no executor config.
> Iffy on how the code on this should look like without making it 
> convoluted because there are a couple of scenarios that need to be handled:
> 
> 1. Task with Docker container and with executor config -> must have a 
> valid config
> 2. Task without docker container and with executor config -> must have a 
> valid config
> 3. Task with Docker container and without executor config -> Accept
> 4. Task without docker container and without executor config -> Reject
> 
> Another question that comes to mind here is, if there is if there's a 
> task with a docker container but no executor config set, should the task be 
> allocated thermo's overhead or not.

Combined with one of Maxim's suggestions, came up with a way to do this. Fixed.


- Renan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review143664
---


On July 26, 2016, 7:04 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 26, 2016, 7:04 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.
> 
> 
> Diffs
> -
> 
>   CHANGELOG fc6a46d77ebf889c8f60402c95e8a7472980ba1c 
>   RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   docs/operations/configuration.md 0615c545f560597683f727c2425de4cc4e82c364 
>   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/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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   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 
> 500fd435b4c72b25abd8df7eea6b3850edc96e99 
>   
> 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 
>   
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-28 Thread Renan DelValle 


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java,
> >  lines 205-208
> > 
> >
> > I'd drop this in favor of EMPTY overhead already returned by the method 
> > call below. It would leave ConfigurationManager and TaskScheduler to assert 
> > both new and existing task routes.

Sounds good to me, applied the change.


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java,
> >  lines 33-34
> > 
> >
> > We usually avoid concrete collection types in publicly accessible 
> > methods/constructors. Accepting Map should be enough. 
> > 
> > If you worry about modifications (which does not seem to be the case 
> > here) you can do ImmutableMap.copyOf() in the assignment.

Makes sense, fixed.


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, 
> > lines 113-120
> > 
> >
> > How about moving EXECUTOR_PREFIX into ExecutorSettings and make it 
> > fully configurable instead? The default would be the current 'thermos-' 
> > value overriden by custom executor definition.

Great suggestion, implemented.


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, 
> > lines 162-164
> > 
> >
> > Is this reachable? Looks like you return Optional.fromNullable() from 
> > that method instead of throwing. 
> > 
> > How about 
> > `executorSettings.getExecutorOverhead(...).orElse(ResourceBag.EMPTY)` 
> > instead? Throwing exception this late in the scheduling cycle does not seem 
> > right anyway.

That sounds like a good idea. I agree that by this point in the pipeline, we 
should have validated everything.


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java,
> >  line 122
> > 
> >
> > This is actually a great finding! Should not be hard to fix it in this 
> > diff and avoid the TODO, e.g.:
> > ```
> > ResourceBag overhead = 
> > executorSettings.getExecutorOverhead(...).orElse(EMPTY);
> > if (tierManager.getTier(victim.getConfig()).isRevocable()) {
> >   bag = 
> > bag.filter(IS_MESOS_REVOCABLE.negate()).add(overhead.filter(IS_MESOS_REVOCABLE.negate()));
> > } else {
> >   bag.add(overhead);
> > }
> > ```

Meant to ask about this but forgot to. Glad I left a todo in there as a 
reminder. Fixed in this diff.


> On July 27, 2016, 1:47 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java, 
> > lines 139-148
> > 
> >
> > I suggest convert this block into an assert statement and move into 
> > current else block:
> > 
> > ```
> > if (!executorSettings.getExecutorConfig(...).isPresent()) {
> >   throw new IllegalStateException("Cannot find executor 
> > configuration...");
> > }
> > ```

Makes sense, changed, thanks for the suggestion. Wasn't sure if it was OK to 
throw an exception from here.


- Renan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review143749
---


On July 26, 2016, 7:04 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 26, 2016, 7:04 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.
> 
> 
> Diffs
> -
> 
>   CHANGELOG fc6a46d77ebf889c8f60402c95e8a7472980ba1c 
>   RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   docs/operations/configuration.md

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-27 Thread Renan DelValle 


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > CHANGELOG, lines 1-4
> > 
> >
> > This file is updated automatically by our release script. You can 
> > revert these changes.

Got it. Was a bit confused because last time my change didn't go into it, but 
it might have to do with the ticket not having been closed at the right time.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java,
> >  line 166
> > 
> >
> > Kill this line?

Good catch, thanks!


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java,
> >  lines 33-34
> > 
> >
> > Fix indentation. Style for continuation indents should be:
> > 
> > public ExecutorSettings(
> > ImmutableMap<...> config,
> > boolean populateDiscoveryInfo) {
> > 
> >   // code continues here after blank line above

Done.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java,
> >  lines 40-45
> > 
> >
> > Instead of creating separate methods for get config and checking a 
> > config's existence, how about changing `getExecutorConfig` to return 
> > `Optional` and using `Optional#isPresent` as the indicator 
> > of whether config exists for that name?

Great suggestion, thanks!


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java,
> >  line 82
> > 
> >
> > House style is: `Map foo = Maps.newHashMap()`. It's a legacy of 
> > the project's pre-Java 7 origins. In theory at some point we could just 
> > move to Java 7 diamond syntax (`... = new HashMap<>()`), but until that 
> > time, best to remain consistent.

Gotcha, will keep this in mind for future patches.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java,
> >  lines 97-98
> > 
> >
> > multiExecutors.put(
> > e.getName(),
> > new ExecutorConfig(...))

Fixed.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 230
> > 
> >
> > Instead of passing in the task, just to use it to get the executor 
> > name, why not just pass in the executor name?

Fixed.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 271
> > 
> >
> > Same here.

For this one, I was passing the task because a valid Docker task may or may not 
have an executor config. I agree though, and an Optional is more suitable here. 
I'll go ahead and make that change.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java, 
> > lines 141-143
> > 
> >
> > Should fit on one line?

Yep! Fixed.


> On July 26, 2016, 8:25 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java, 
> > line 158
> > 
> >
> > Are we guaranteed that the executor config exists here? The previous 
> > branch is checking if executorConfig is set and that the config exists, but 
> > won't we fall into this branch if executorConfig is not set?

Great catch! I forgot to put in some logic to handle the case where it's a 
Docker task with no executor config.
Iffy on how the code on this should look like without making it convoluted 
because there are a couple of scenarios that need to be handled:

1. Task with Docker container and with executor config -> must have a valid 
config
2. Task without docker container and with executor config -> must have a valid 
config
3. Task with Docker container and without executor config -> Accept
4. Task without docker container and without executor config -> Reject

Another question that comes to mind here is, if there is if there's a task with 
a docker container but no executor config set, should the task be allocated 
thermo's overhead or not.


> On

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-27 Thread Maxim Khutornenko 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review143749
---




src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
 (lines 33 - 34)


We usually avoid concrete collection types in publicly accessible 
methods/constructors. Accepting Map should be enough. 

If you worry about modifications (which does not seem to be the case here) 
you can do ImmutableMap.copyOf() in the assignment.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
113 - 120)


How about moving EXECUTOR_PREFIX into ExecutorSettings and make it fully 
configurable instead? The default would be the current 'thermos-' value 
overriden by custom executor definition.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (lines 
162 - 164)


Is this reachable? Looks like you return Optional.fromNullable() from that 
method instead of throwing. 

How about 
`executorSettings.getExecutorOverhead(...).orElse(ResourceBag.EMPTY)` instead? 
Throwing exception this late in the scheduling cycle does not seem right anyway.



src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
(line 122)


This is actually a great finding! Should not be hard to fix it in this diff 
and avoid the TODO, e.g.:
```
ResourceBag overhead = 
executorSettings.getExecutorOverhead(...).orElse(EMPTY);
if (tierManager.getTier(victim.getConfig()).isRevocable()) {
  bag = 
bag.filter(IS_MESOS_REVOCABLE.negate()).add(overhead.filter(IS_MESOS_REVOCABLE.negate()));
} else {
  bag.add(overhead);
}
```



src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
(lines 205 - 208)


I'd drop this in favor of EMPTY overhead already returned by the method 
call below. It would leave ConfigurationManager and TaskScheduler to assert 
both new and existing task routes.



src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java (lines 
139 - 148)


I suggest convert this block into an assert statement and move into current 
else block:

```
if (!executorSettings.getExecutorConfig(...).isPresent()) {
  throw new IllegalStateException("Cannot find executor configuration...");
}
```


- Maxim Khutornenko


On July 27, 2016, 2:04 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 27, 2016, 2:04 a.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.
> 
> 
> Diffs
> -
> 
>   CHANGELOG fc6a46d77ebf889c8f60402c95e8a7472980ba1c 
>   RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   docs/operations/configuration.md 0615c545f560597683f727c2425de4cc4e82c364 
>   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/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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 1dfa97e659c2fca8308cb592f37d14328e4b42bc 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  ee6fe95eaa41c7952a974ebea069b3de6ed82994 
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-26 Thread Joshua Cohen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review143664
---




CHANGELOG (lines 1 - 4)


This file is updated automatically by our release script. You can revert 
these changes.



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java
 (line 165)


Kill this line?



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
 (lines 33 - 34)


Fix indentation. Style for continuation indents should be:

public ExecutorSettings(
ImmutableMap<...> config,
boolean populateDiscoveryInfo) {

  // code continues here after blank line above



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java
 (lines 40 - 45)


Instead of creating separate methods for get config and checking a config's 
existence, how about changing `getExecutorConfig` to return 
`Optional` and using `Optional#isPresent` as the indicator of 
whether config exists for that name?



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
 (line 61)


s/An map/A map



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
 (line 82)


House style is: `Map foo = Maps.newHashMap()`. It's a legacy of the 
project's pre-Java 7 origins. In theory at some point we could just move to 
Java 7 diamond syntax (`... = new HashMap<>()`), but until that time, best to 
remain consistent.



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
 (lines 97 - 98)


multiExecutors.put(
e.getName(),
new ExecutorConfig(...))



src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java
 (line 100)


We should probably return an `ImmutableMap` here.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 230)


Instead of passing in the task, just to use it to get the executor name, 
why not just pass in the executor name?



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 271)


Same here.



src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java (lines 
141 - 143)


Should fit on one line?



src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java (line 
158)


Are we guaranteed that the executor config exists here? The previous branch 
is checking if executorConfig is set and that the config exists, but won't we 
fall into this branch if executorConfig is not set?


- Joshua Cohen


On July 27, 2016, 2:04 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 27, 2016, 2:04 a.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.
> 
> 
> Diffs
> -
> 
>   CHANGELOG fc6a46d77ebf889c8f60402c95e8a7472980ba1c 
>   RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   docs/operations/configuration.md 0615c545f560597683f727c2425de4cc4e82c364 
>   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/ExecutorModule.java
>  8b7f8dcf4d8e0372208e636b3f57fc159272ecc7 
>   
>

Re: Review Request 50480: Multiple executor support in Scheduler

2016-07-26 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50480/#review143653
---


Ship it!




Master (08792d4) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 27, 2016, 2:04 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50480/
> ---
> 
> (Updated July 27, 2016, 2:04 a.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.
> 
> 
> Diffs
> -
> 
>   CHANGELOG fc6a46d77ebf889c8f60402c95e8a7472980ba1c 
>   RELEASE-NOTES.md d46420ef876b88d9ab68c4816f1c2d6780aba702 
>   docs/operations/configuration.md 0615c545f560597683f727c2425de4cc4e82c364 
>   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/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 
> cbbd6be94aa857b02cd7b45bfb2f0216d9a1cec3 
>   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 
> 500fd435b4c72b25abd8df7eea6b3850edc96e99 
>   
> 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
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>