You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/01/07 01:26:13 UTC
Re: Review Request 28617: Implemented offer filtering for tasks with
static vetoes.
> On Dec. 24, 2014, 12:26 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/TaskVars.java, line 79
> > <https://reviews.apache.org/r/28617/diff/4/?file=790574#file790574line79>
> >
> > How about either
> >
> > 1) Make this a member function of VetoGroup (so it's guaranteed by the compiler to be implemented if we add a new group type).
> >
> > 2) Make this a Function<VetoGroup, Optional<String>>.
> >
> > 3) Make this a Function<VetoGroup, String> with a default for a missing value.
> >
> > Right now we risk a runtime NullPointerException if we add a new VetoGroup implementation and forget to update this map in a different package.
#1 - metric name mapping does not logically fit into the VetoGroup
#2 - this would add extra heap churn (Optional creation) I am trying to avoid for this very high frequency call event handler
#3 - default string value would slip the build phase and manifest itself as an invalid metric name at runtime, which is arguably even worse.
How about a test case exhaustively covering all VetoGroup mappings instead? Added.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28617/#review65988
-----------------------------------------------------------
On Dec. 15, 2014, 11:12 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28617/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2014, 11:12 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-909
> https://issues.apache.org/jira/browse/AURORA-909
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Modified the task offer/task matching logic to skip offer matching for tasks previously vetoed statically.
>
> Preliminary testing in vagrant (see pictures below) shows close to 50% improvement in task scheduling performance.
>
> Update:
> Testing with JMH (https://reviews.apache.org/r/28710/ and https://reviews.apache.org/r/28731/) shows over 97% better perf when testing with disabled preemptor:
> ```
> Master with cluster fillup 0.9:
> Benchmark Mode Samples Score Error Units
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 8291046.074 ± 145251.995 ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 7522269.050 ± 142446.265 ns/op
>
> This RB with cluster fillup 0.9:
> Benchmark Mode Samples Score Error Units
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.example avgt 100 204171.046 ± 3800.124 ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.example avgt 100 215854.129 ± 8959.851 ns/op
> ```
>
> Testing with preemptor enabled and running the worst case possible scenario (every slave is eligible and has task victims to evaluate) is still 16-22% faster than the master.
> ```
> Master with cluster fillup 0.9:
> Benchmark Mode Samples Score Error Units
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 100 7170244.522 ± 230259.848 ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 8158184.909 ± 465853.379 ns/op
>
> This RB with cluster fillup 0.9:
> Benchmark Mode Samples Score Error Units
> o.a.a.b.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark avgt 100 6050901.381 ± 182134.371 ns/op
> o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100 6425953.419 ± 163741.064 ns/op
> ```
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/TaskVars.java f017cdd26ca40138a7e141f21613ed567314c399
> src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java f66383830140e5eaba436f35ebb5192eee65947a
> src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e1c29747c9854cf75bf63f6f085cf40ca68989af
> src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 4e7efb3c1214c3d193afd61f162713490eb8effb
> src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 4cf602ad32b972c18eb5a81e9b2f59c67859bdb2
> src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 5647349854a5e04de749c4d809684a0066d4da06
> src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 6cc13231560996b144101eba36577f49017aba06
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949
> src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 411a55a8d85f60bb2703468f2d69b64b2736eee4
>
> Diff: https://reviews.apache.org/r/28617/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
> Tested in vagrant
>
>
> File Attachments
> ----------------
>
> NoStaticVetoFiltering.png
> https://reviews.apache.org/media/uploaded/files/2014/12/03/7945c60b-4135-4016-a9bf-8d4815a4a573__NoStaticVetoFiltering.png
> StaticVetoFiltering.png
> https://reviews.apache.org/media/uploaded/files/2014/12/03/2f73b94a-5ba9-43b6-922e-e9e4ec18d0bb__StaticVetoFiltering.png
>
>
> Thanks,
>
> Maxim Khutornenko
>
>