> On May 12, 2017, 2:20 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java
> > Lines 81 (patched)
> > <https://reviews.apache.org/r/59039/diff/2/?file=1716093#file1716093line81>
> >
> >     How does this look like on the commandline? Do I have to provide a 
> > fully-specified class name here?

Yup.


> On May 12, 2017, 2:20 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java
> > Line 130 (original), 174 (patched)
> > <https://reviews.apache.org/r/59039/diff/2/?file=1716093#file1716093line178>
> >
> >     Changes like these and others in this patch could potentially break our 
> > benchmarking scripts.
> >     
> >     Could you please make sure that at least the `SchedulingBenchmarks` 
> > continue to run?
> >     
> >     We already have several broken the benchmarks suites 
> > `ThriftApiBenchmarks`, `SnapshotBenchmarks` `StatusUpdateBenchmark` and 
> > `ThriftApiBenchmarks`. Would be a pitty if we break even more.
> 
> Joshua Cohen wrote:
>     Not directly related here, but should we add something that regularly 
> runs the benchmarks and alerts if they stop working (or if there's a major 
> regression in terms of perf)?
> 
> Stephan Erb wrote:
>     Good idea! We might need some tweaking of the benchmark footprint but 
> that should be feasible.
> 
> David McLaughlin wrote:
>     Do the SchedulingBenchmarks really pass for you on master? I spent a 
> while trying to debug the broken scheduling benchmarks.. but going back a 
> month of commits they seem broken then too. This issue is making it really 
> difficult to figure out the missing guice bindings: 
> https://github.com/google/guice/issues/757
> 
> David McLaughlin wrote:
>     AFAICT, they've been broken since this commit: 
> https://github.com/apache/aurora/commit/c32f14c75e7f8620aedb12d38aee8ca1d2427724
>     
>     I'll try and fix them in a separate patch.

These are now running.


> On May 12, 2017, 2:20 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/state/FirstFitTaskAssignerModule.java
> > Lines 28 (patched)
> > <https://reviews.apache.org/r/59039/diff/2/?file=1716094#file1716094line28>
> >
> >     For consistency how about renaming `TaskAssignerImpl` to 
> > `FirstFitTaskAssignerImpl`?

Done.


- David


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


On May 25, 2017, 5:50 a.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59039/
> -----------------------------------------------------------
> 
> (Updated May 25, 2017, 5:50 a.m.)
> 
> 
> Review request for Aurora, Santhosh Kumar Shanmugham, Stephan Erb, and Zameer 
> Manji.
> 
> 
> Bugs: AURORA-1920
>     https://issues.apache.org/jira/browse/AURORA-1920
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add the ability to replace the first-fit scheduling algorithm and associated 
> first-fit preemption logic.
> 
> See design/proposal document here: 
> https://docs.google.com/document/d/1fVHLt9AF-YbOCVCDMQmi5DATVusn-tqY8DldKbjVEm0/edit?usp=sharing
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 77376e438bd7af74c364dcd5d1b3e3f1ece2adbf 
>   docs/development/design-documents.md 
> c942643d6220cb61eb97289b35d92facae06a682 
>   docs/reference/scheduler-configuration.md 
> 3e3d7991970fc75907d07c5a8a31d508c26eda78 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorModule.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterModule.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java 
> 92087eb60cb81e1c64eaa7d1e2da7f6bbd51500c 
>   
> src/main/java/org/apache/aurora/scheduler/state/FirstFitTaskAssignerModule.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/state/StateModule.java 
> 0186484208965c5b8942ec76eb078b30190c7d57 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> 7c531afee880022af5db0fa860267a96bc735dde 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 
> da064e3589951d4f4cbff96d1ed89d9d7e4d3882 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 11835dc21ffd649b04867fcbf14e5003b6d7f7aa 
> 
> 
> Diff: https://reviews.apache.org/r/59039/diff/3/
> 
> 
> Testing
> -------
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> I've used this in one of our large-scale test clusters to plug in a custom 
> task assigner. We were able to remove our modifications to the OSS code-base 
> and use this mechanism to do what we need. Here is a rough gist of what it 
> looks like:
> https://gist.github.com/DavidMcLaughlin/5d5472cde15343aef8705478f644ddb9
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

Reply via email to