-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63973/#review191709
-----------------------------------------------------------
Master (0f3dc93) is red with this patch.
./build-support/jenkins/build.sh
symbol: variable OUTSTANDING_OFFERS
location: class OfferManagerImplTest
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java:206:
error: cannot find symbol
offerManager.addOffer(OFFER_A);
^
symbol: method addOffer(HostOffer)
location: variable offerManager of type OfferManagerImpl
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java:207:
error: cannot find symbol
assertEquals(0, statsProvider.getLongValue(OUTSTANDING_OFFERS));
^
symbol: variable OUTSTANDING_OFFERS
location: class OfferManagerImplTest
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java:214:
error: cannot find symbol
offerManager.addOffer(OFFER_A);
^
symbol: method addOffer(HostOffer)
location: variable offerManager of type OfferManagerImpl
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java:215:
error: cannot find symbol
assertEquals(OFFER_A, Iterables.getOnlyElement(offerManager.getOffers()));
^
symbol: method getOffers()
location: variable offerManager of type OfferManagerImpl
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
100 errors
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':compileTestJava'.
> Compilation failed; see the compiler error output for details.
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug
option to get more log output.
* Get more help at https://help.gradle.org
BUILD FAILED in 3m 44s
27 actionable tasks: 21 executed, 6 up-to-date
I will refresh this build result if you post a review containing "@ReviewBot
retry"
- Aurora ReviewBot
On Nov. 22, 2017, 8:38 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63973/
> -----------------------------------------------------------
>
> (Updated Nov. 22, 2017, 8:38 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham,
> Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This is a rather large diff. Posting without tests first to get initial
> impressions/make the review more digestible, but tests are soon to come.
>
> Major portions of the refactor:
>
> * Allows injection of custom `OfferManager` via `OfferManagerModule`
> * Refactor `OfferManager` to do filtering of offers (added `getMatching` and
> `getAllMatching` methods) as opposed to TaskAssigner
> * Refactor `TaskAssigner`, allow for injection of custom "scoring" class
> through `OfferRanker` interface
>
> And some minor things as well:
>
> * Moved `TaskAssignerImpl`, `TaskSchedulerImpl`, and `HostOffers` into their
> own upper-level classes
> * Moved `TaskAssigner` to the `scheduling` package and out of the `state`
> package
> * Renamed some methods in `OfferManager` to avoid code stutter
> * Renaming of some classes (e.g. `FirstFitTaskAssigner` -> `TaskAssignerImpl`)
> * And a slew of others
>
> Overall, the goal of this refactor is to allow for the easy injection of
> custom offer "scoring" modules via the `OfferRanker` interface. Callers will
> not have to replace the entire `TaskAssigner` module, but instead just
> provide a function that chooses from a list of offers that should already fit
> the task.
>
> Going through this review, I would start with the
> `maybeAssign`/`launchUsingOffer` methods in `TaskAssigner`. Then, I would
> look at `getMatching`/`getAllMatching` in `HostOffers`.
>
>
> Diffs
> -----
>
> docs/reference/scheduler-configuration.md
> d17541f9458650240983276b69f749a854057aa8
> src/main/java/org/apache/aurora/scheduler/app/AppModule.java
> 3204cca5446d0f798ace622d334473bf6db13733
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java
> d4537e386af53ec9bff661ed23d61a5bd4893edf
> src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
> 60f141d34d862f200ae1346a38a71bba57551059
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
> 36608a9f027c95723c31f9915852112beb367223
> src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
> df51d4cf4893899613683603ab4aa9aefa88faa6
> src/main/java/org/apache/aurora/scheduler/http/Offers.java
> f22ca6e5402d8ec7241376de14a9051ab0cfbfc6
> src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java
> fd5874d51cfbca1b98d15623a9c845d208d43b1f
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
> 96b0f46e69255944fa0f00621db315e22d68515c
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerImpl.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
> 57fc1a18f1b82decc780ba0069ab4dd0c125f77d
> src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java
> 4a6ea8dac97ed2f5df47e4d16fa65a3b516988bd
>
> src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java
> 766d3b24fd64de96b82d3dd2a17f7a80331889d8
> src/main/java/org/apache/aurora/scheduler/preemptor/Preemptor.java
> 82a0ff67e9f70fed305f8f1bf38e3cf7955b9ce7
>
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferRanker.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferRanker.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/scheduling/SchedulingModule.java
> 0796712a00e3cba595ceb049833933b9a70c2f9a
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssigner.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
> PRE-CREATION
>
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskScheduler.java
> 0002b0ceb429a404ac9dd0ae7e5ea9f6362fa100
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImpl.java
> PRE-CREATION
>
> src/main/java/org/apache/aurora/scheduler/state/FirstFitTaskAssignerModule.java
> dc244eeaa0d292a06d8750cb340fb379701ad49f
> src/main/java/org/apache/aurora/scheduler/state/StateModule.java
> c03fff11ea3a4086f9daaa8b07315006c1b481e4
> src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
> cdd0d15872248b19d5eb52d25537f2f863fd5c77
> src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java
> 6033c01477bec7a491128e3089ffc1b1c3f1c150
>
>
> Diff: https://reviews.apache.org/r/63973/diff/2/
>
>
> Testing
> -------
>
> Tests coming soon, but they should be relatively close to what we have now on
> master since we don't really add/remove features but just move them around.
>
> Ran some benchmarks as well to ensure performance parity:
>
> ```
> MASTER
> Benchmark
> Mode Cnt Score Error Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark
> thrpt 10 113910.922 ± 26263.903 ops/s
> SchedulingBenchmarks.FillClusterBenchmark.runBenchmark
> thrpt 10 263.043 ± 299.686 ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
> thrpt 10 12353.669 ± 433.182 ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
> thrpt 10 893.263 ± 54.882 ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
> thrpt 10 11478.420 ± 1068.819 ops/s
> ```
> and
> ```
> MY PATCH
> Benchmark
> Mode Cnt Score Error Units
> SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark
> thrpt 10 124041.966 ± 33331.424 ops/s
> SchedulingBenchmarks.FillClusterBenchmark.runBenchmark
> thrpt 10 1030.434 ± 1223.539 ops/s
> SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark
> thrpt 10 12305.154 ± 375.814 ops/s
> SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
> thrpt 10 1080.267 ± 74.635 ops/s
> SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
> thrpt 10 11604.642 ± 586.248 ops/s
> ```
>
> Not exactly sure why `FillClusterBenchmark` increased so dramatically -- I
> will look into that while testing as well.
>
>
> Thanks,
>
> Jordan Ly
>
>
