You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Renan DelValle <rd...@binghamton.edu> on 2015/08/18 02:28:50 UTC

Re: Review Request 36289: Custom executor support for Scheduler

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

(Updated Aug. 18, 2015, 12:28 a.m.)


Review request for Aurora and Bill Farner.


Changes
-------

Added support for dynamically chosing an executor via the name provided in the thrift call using MapBinder.
Mesos-command executor now pulls command from data field inside of executor settings in the thrift call.
Removed executor name as an argument.
Changed code to allow GSON to do the heavy lifting.
Updated test classes to reflect the use of a Map instead of a single instance of ExecutorSettings.

Tested using different executors on one scheduler simultaneously using a modified version of the client.


Repository: aurora


Description
-------

What was done:
==============
Added support for custom executors in the Scheduler via a config file. 
Removed command line arguments that were moved over to the config file.

Future:
=======
Extending the client to support custom executors and the mesos-executor.

Caveats:
========
This contains initial config file with support for thermos and limited support for the mesos commandline executor. Mesos-command line executor needs support from the client side in order to function at a better capacity. 

Currently, this uses the current client to launch both tasks, meaning as long as the client sends a thrift call, the scheduler will schedule a task, be it a mesos-command task with a preconfigured command temporarily set in the config file or a custom executor task. 

*Support for custom executors in the client must be added in order to fully utilize this feature.*


Diffs (updated)
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
  src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
  src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 

Diff: https://reviews.apache.org/r/36289/diff/


Testing
-------

Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.


Thanks,

Renan DelValle


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Renan DelValle <rd...@binghamton.edu>.

> On Aug. 18, 2015, 3:06 a.m., Aurora ReviewBot wrote:
> > Master (22f9cbb) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > 
> > :api:checkPython
> > :api:generateThriftEntitiesJava
> > :api:classesThriftEntities
> > :api:compileJava UP-TO-DATE
> > :api:generateThriftResources
> > :api:processResources UP-TO-DATE
> > :api:classes
> > :api:jar
> > :compileJavaNote: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2
> > 
> > :processResources
> > :classes
> > :jar
> > :startScripts
> > :distTar
> > :distZip
> > :assemble
> > :compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
> > Note: Recompile with -Xlint:deprecation for details.
> > 
> > :processJmhResources UP-TO-DATE
> > :jmhClasses
> > :checkstyleJmh
> > :jsHint
> > :checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java:23:8: Unused import - com.google.gson.JsonObject.
> >  FAILED
> > 
> > FAILURE: Build failed with an exception.
> > 
> > * What went wrong:
> > Execution failed for task ':checkstyleMain'.
> > > Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml
> > 
> > * Try:
> > Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
> > 
> > BUILD FAILED
> > 
> > Total time: 1 mins 35.518 secs
> > 
> > 
> > I will refresh this build result if you post a review containing "@ReviewBot retry"

@ReviewBot retry


- Renan


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


On Aug. 18, 2015, 6:10 a.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 6:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> What was done:
> ==============
> Added support for dynamically chosing an executor that's definied in a server side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.
> 
> Future:
> =======
> Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
>   src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> -------
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/#review95680
-----------------------------------------------------------


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


:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java:23:8: Unused import - com.google.gson.JsonObject.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Total time: 1 mins 35.518 secs


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

- Aurora ReviewBot


On Aug. 18, 2015, 12:33 a.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 12:33 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> What was done:
> ==============
> Added support for dynamically chosing an executor that's definied in a server side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.
> 
> Future:
> =======
> Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
>   src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> -------
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/#review95873
-----------------------------------------------------------


Partial review - stopped at the question of how to ensure the precondition check added in SchedulingFilterImpl always passes. I think this patch needs to add startup validation and a backfill strategy.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 46)
<https://reviews.apache.org/r/36289/#comment151027>

    Hmm - why does this magic string exist? Any idea what the implications of changing it are?



examples/vagrant/executors-config.json (lines 5 - 6)
<https://reviews.apache.org/r/36289/#comment151028>

    Is there a difference between path and resources as far as Mesos is concerned?



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 34)
<https://reviews.apache.org/r/36289/#comment151037>

    Given that you use this type in a `Set` you will need to define `hashCode` and `equals` here (and for all contained types).



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 37)
<https://reviews.apache.org/r/36289/#comment151034>

    Make these `final` and add a constructor for testing?



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 48)
<https://reviews.apache.org/r/36289/#comment151038>

    Needs `hashCode` and `equals`



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 85)
<https://reviews.apache.org/r/36289/#comment151031>

    remove redundant use of `this`, here and below



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (lines 40 - 44)
<https://reviews.apache.org/r/36289/#comment151036>

    No JavaDoc for private methods - use a comment instead:
    
    ```java
    private ExecutorSettingsLoader() {
      // Utility class.
    }
    ```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 60)
<https://reviews.apache.org/r/36289/#comment151035>

    Take a `File` here instead of a `String`.
    
    Return a narrower type, like `Set<ExecutorSettings>` (but probably `List<ExecutorSetttings>`, see below.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 63)
<https://reviews.apache.org/r/36289/#comment151039>

    Prefer `ImmutableSet.Builder`



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 66)
<https://reviews.apache.org/r/36289/#comment151040>

    You leak a file descriptor here - consider just using `Files.asCharSource(configFile, StandardCharsets.UTF_8)` and opening it in a try-with-resources block:
    
    ```java
    CharSource configFileSource = Files.asCharSource(configFile, StandardCharsets.UTF_8);
    try (Reader reader = configFileSource.openBufferedReader()) {
      gson.read(...);
    }
    ```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 68)
<https://reviews.apache.org/r/36289/#comment151047>

    nit: no space between `}` and `.`



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 75)
<https://reviews.apache.org/r/36289/#comment151046>

    Mentioning the filename would make this easier to debug.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (lines 76 - 77)
<https://reviews.apache.org/r/36289/#comment151044>

    This is actually caused by the platform JVM not having support for an encoding (which is impossible for UTF8 as it's required by the standard). Use the `asCharSource` suggestion with the constant from `StandardCharsets` to avoid needing to handle this.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 79)
<https://reviews.apache.org/r/36289/#comment151045>

    You'll want to concat the error message here as well as the second param will just print the stack trace. Also a bit more context would be useful. e.g.
    
    ```java
    throw new ExecutorSettingsException("Error parsing executor settings JSON: " + e, e);
    ```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 81)
<https://reviews.apache.org/r/36289/#comment151041>

    Can you determine which parameter is missing? This would be a very frustrating error message to debug.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 84)
<https://reviews.apache.org/r/36289/#comment151048>

    You read this as a `List`, any reason not to use `ImmutableList.copyOf` here instead?



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (line 81)
<https://reviews.apache.org/r/36289/#comment151049>

    `Arg<File>` and add a `@CanRead` annotation for more useful validation.



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (line 170)
<https://reviews.apache.org/r/36289/#comment151050>

    This needs to re-throw, otherwise initialization will continue and users will experience weird runtime behavior. Consider removing the custom exception type and throwing `IllegalArgumentException` in the parser.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java (lines 211 - 213)
<https://reviews.apache.org/r/36289/#comment151051>

    Formatting is weird - push the call chain to the next line:
    
    ```java
    ResourceSlot.from(request.getTask())
        .withOverhead(requireNonNull(executorSettings.get(task.getExecutorConfig().getName());
    ```
    
    This is a weird (though correct) place for a `requireNonNull` check though because the scheduler can crash in its scheduling loop in the case of a config change.
    
    I think you need to add logic to make sure that every task has an executor that's acceptible to the system at startup.


- Kevin Sweeney


On Aug. 17, 2015, 11:10 p.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2015, 11:10 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> What was done:
> ==============
> Added support for dynamically chosing an executor that's definied in a server side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.
> 
> Future:
> =======
> Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
>   src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> -------
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>


Re: Review Request 36289: Custom executor support for Scheduler

Posted by John Sirois <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/#review107128
-----------------------------------------------------------


This has been idle a good while and it appears wfarner has picked up work on https://issues.apache.org/jira/browse/AURORA-1376, is it safe to discard?

- John Sirois


On Aug. 18, 2015, 12:10 a.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 12:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> What was done:
> ==============
> Added support for dynamically chosing an executor that's definied in a server side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.
> 
> Future:
> =======
> Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
>   src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> -------
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/#review95689
-----------------------------------------------------------


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

                     src.test.python.apache.aurora.client.cli.plugins                                .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.quota                                  .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.sla                                    .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.supdate                                .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.task                                   .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.update                                 .....   SUCCESS
                     src.test.python.apache.aurora.client.cli.version                                .....   SUCCESS
                     src.test.python.apache.aurora.client.config                                     .....   SUCCESS
                     src.test.python.apache.aurora.client.hooks.hooked_api                           .....   SUCCESS
                     src.test.python.apache.aurora.client.hooks.non_hooked_api                       .....   SUCCESS
                     src.test.python.apache.aurora.common.test_aurora_job_key                        .....   SUCCESS
                     src.test.python.apache.aurora.common.test_cluster                               .....   SUCCESS
                     src.test.python.apache.aurora.common.test_cluster_option                        .....   SUCCESS
                     src.test.python.apache.aurora.common.test_clusters                              .....   SUCCESS
                     src.test.python.apache.aurora.common.test_http_signaler                         .....   SUCCESS
                     src.test.python.apache.aurora.common.test_pex_version                           .....   SUCCESS
                     src.test.python.apache.aurora.common.test_shellify                              .....   SUCCESS
                     src.test.python.apache.aurora.common.test_transport                             .....   SUCCESS
                     src.test.python.apache.aurora.config.test_base                                  .....   SUCCESS
                     src.test.python.apache.aurora.config.test_constraint_parsing                    .....   SUCCESS
                     src.test.python.apache.aurora.config.test_loader                                .....   SUCCESS
                     src.test.python.apache.aurora.config.test_thrift                                .....   SUCCESS
                     src.test.python.apache.aurora.executor.common.path_detector                     .....   SUCCESS
                     src.test.python.apache.aurora.executor.common.task_info                         .....   SUCCESS
                     src.test.python.apache.aurora.executor.executor_base                            .....   SUCCESS
                     src.test.python.apache.aurora.executor.executor_vars                            .....   SUCCESS
                     src.test.python.apache.aurora.executor.http_lifecycle                           .....   SUCCESS
                     src.test.python.apache.aurora.executor.status_manager                           .....   SUCCESS
                     src.test.python.apache.aurora.executor.thermos_task_runner                      .....   FAILURE
                     src.test.python.apache.thermos.cli.commands.commands                            .....   SUCCESS
                     src.test.python.apache.thermos.cli.common                                       .....   SUCCESS
                     src.test.python.apache.thermos.cli.main                                         .....   SUCCESS
                     src.test.python.apache.thermos.common.test_pathspec                             .....   SUCCESS
                     src.test.python.apache.thermos.core.test_runner_integration                     .....   SUCCESS
                     src.test.python.apache.thermos.monitoring.test_disk                             .....   SUCCESS
                     
FAILURE


               FAILURE


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

- Aurora ReviewBot


On Aug. 18, 2015, 6:10 a.m., Renan DelValle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 6:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> What was done:
> ==============
> Added support for dynamically chosing an executor that's definied in a server side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.
> 
> Future:
> =======
> Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
>   src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
>   src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> -------
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Renan DelValle <rd...@binghamton.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/
-----------------------------------------------------------

(Updated Aug. 18, 2015, 6:10 a.m.)


Review request for Aurora and Bill Farner.


Changes
-------

Commited unused import that I had forgotten to :B


Repository: aurora


Description
-------

What was done:
==============
Added support for dynamically chosing an executor that's definied in a server side config file.
Removed command line arguments that were moved over to the config file.
Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.

Future:
=======
Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.


Diffs (updated)
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
  src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
  src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 

Diff: https://reviews.apache.org/r/36289/diff/


Testing
-------

Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
Ran end to end, failed upon reaching kerberos tests.


Thanks,

Renan DelValle


Re: Review Request 36289: Custom executor support for Scheduler

Posted by Renan DelValle <rd...@binghamton.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36289/
-----------------------------------------------------------

(Updated Aug. 18, 2015, 12:33 a.m.)


Review request for Aurora and Bill Farner.


Repository: aurora


Description (updated)
-------

What was done:
==============
Added support for dynamically chosing an executor that's definied in a server side config file.
Removed command line arguments that were moved over to the config file.
Updated existing code to reflect the use of a Map instead of a single ExecutorSettings object.

Future:
=======
Create an offshoot of the current client that allows to send thrift calls with different executor configs which will allow use of custom executors. Some work on this has already been done and will be published ASAP for testing.


Diffs
-----

  api/src/main/thrift/org/apache/aurora/gen/api.thrift f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 8162323816aedc711a3af84cd499250b78718ab3 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 37772d0b75d022f072af10d82d096981680e193f 
  src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 608af1afe6fc27c8c597490e88fed75580076c95 
  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java a1ac922d471013779710e02c0c9ca9f84b506807 
  src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java b07ff7babd217dac4153831a0d78325bcb72b306 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json PRE-CREATION 
  src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json PRE-CREATION 

Diff: https://reviews.apache.org/r/36289/diff/


Testing (updated)
-------

Ran jenkins build test, passed all tests, code style checks, findbugs check, and PMD.
Ran end to end, failed upon reaching kerberos tests.


Thanks,

Renan DelValle