----------------------------------------------------------- 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. Changes ------- Responded to comments, removed injectability of `OfferManager` 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 (updated) ----- 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/ Changes: https://reviews.apache.org/r/63973/diff/1-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
