----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63973/#review191992 -----------------------------------------------------------
Ship it! Ship It! - David McLaughlin On Nov. 27, 2017, 7:41 p.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63973/ > ----------------------------------------------------------- > > (Updated Nov. 27, 2017, 7:41 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, > Stephan Erb, and Bill Farner. > > > Repository: aurora > > > Description > ------- > > Major portions of the refactor: > > * 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 > ----- > > RELEASE-NOTES.md e622a8d38425a99c2965d64147fa0e680e5de517 > docs/reference/scheduler-configuration.md > 6c385b5be2b64117989c624ce93c78609cfea6c5 > src/jmh/java/org/apache/aurora/benchmark/Offers.java > 2b463267af6e7ae7fa16736de3b1e3d7d5bd99a8 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 1708a5063afd12de5d2138e763a19d1cf8dc9714 > src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java > 201aa81db7f607a70c7d44011857576f45b7e9da > 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/FirstFitOfferSelector.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.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 > b7a3c0be592c13181edf2e8c241a2ad351962e06 > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java > cdd0d15872248b19d5eb52d25537f2f863fd5c77 > src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java > 6033c01477bec7a491128e3089ffc1b1c3f1c150 > src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java > 0ec4de636e2fa0b671f8c1368e6d225747f215b6 > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java > 30699596a1c95199df7504f62c5c18cab1be1c6c > > src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java > 45ae6bb5d12de66271a99aad0cda9632da22acea > src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java > 6b18296ffdd7e1a5e989236db967c5e752a8a55f > > src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java > c76b3e338298d63834b4265d769649d39b9ae219 > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java > f14cba144856b23531467a7b8a3fd695ad8beee8 > > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java > 97c238ce30142a94a1fd9c6c7055fa95f2512cce > > src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImplTest.java > 7a4525a190a662e82b9ba98346fae0b25a8ea673 > > src/test/java/org/apache/aurora/scheduler/state/FirstFitTaskAssignerTest.java > 58f9de2731633664f4140d785c3a6df0cde063b4 > src/test/java/org/apache/aurora/scheduler/stats/AsyncStatsModuleTest.java > f8e802342e753ee6a23ba7e1c2ab5151b85cadb3 > > > Diff: https://reviews.apache.org/r/63973/diff/5/ > > > Testing > ------- > > Ran some benchmarks as well to ensure performance parity: > > ``` > MASTER > Benchmark > Mode Cnt Score Error Units > SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark > thrpt 10 138060.371 ± 17807.792 ops/s > SchedulingBenchmarks.FillClusterBenchmark.runBenchmark > thrpt 10 847.428 ± 1144.232 ops/s > SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark > thrpt 10 12484.444 ± 1788.357 ops/s > SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark > thrpt 10 905.377 ± 356.045 ops/s > SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark > thrpt 10 11973.847 ± 1703.385 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 > ``` > > Edit 11/27: reran master and got similar benchmarks. Probably a difference in > environment caused the original disparity. > > > Thanks, > > Jordan Ly > >
