aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jordan Ly <jordan....@gmail.com>
Subject Re: Review Request 63973: Enable custom offer scoring modules for task assignment
Date Mon, 27 Nov 2017 19:41:57 GMT

-----------------------------------------------------------
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.


Changes
-------

Renamed OfferRanker -> OfferSelector, removed extra args from OfferSelector


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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/63973/diff/4-5/


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message