aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Farner <wfar...@apache.org>
Subject Re: Review Request 63973: Enable custom offer scoring modules for task assignment and injecting of custom OfferManagers
Date Tue, 21 Nov 2017 20:53:16 GMT


> On Nov. 20, 2017, 7:55 p.m., Jordan Ly wrote:
> > src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
> > Line 52 (original)
> > <https://reviews.apache.org/r/63973/diff/1/?file=1898144#file1898144line52>
> >
> >     Note for reviewers: this was causing some tests to fail due to my refactors
not calling the initialization.
> >     
> >     I removed this optimization since it seemed to be geared toward the internal
DB implementation we just removed. The benchmarks look alright, but I may be overlooking something
from the original issue.
> >     
> >     I was looking at this patch as context: https://reviews.apache.org/r/53918/

Unfortunate if not covered in tests, but this appears to be necessary for correctness when
multiple tasks are scheduled per scheduling round.  This seems to be the review for meaningful
context: https://reviews.apache.org/r/51929/

When a task is successfully scheduled in a scheduling round, the AttributeAggregate is updated
so that it remains correct in a subsequent attempt within the same round.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63973/#review191561
-----------------------------------------------------------


On Nov. 20, 2017, 7:40 p.m., Jordan Ly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63973/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2017, 7:40 p.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/OfferManagerModule.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/1/
> 
> 
> 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