You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Jordan Ly <jo...@gmail.com> on 2018/01/19 06:05:28 UTC
Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/
-----------------------------------------------------------
Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
Repository: aurora
Description
-------
The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
The following options will be removed from the last 0.19 to now:
```
-offer_order
-offer_order_modules
```
Diffs
-----
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
Diff: https://reviews.apache.org/r/65233/diff/1/
Testing
-------
`./gradlew test`
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
Testing injection of a custom `OfferSet` onto a test cluster.
Thanks,
Jordan Ly
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195840
-----------------------------------------------------------
Ship it!
Master (c4c55ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 6:11 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 6:11 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/2/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 19, 2018, 10:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
> > Line 66 (original), 65 (patched)
> > <https://reviews.apache.org/r/65233/diff/1/?file=1942312#file1942312line66>
> >
> > Inject `OfferSet` rather than `OfferSettings`.
>
> Jordan Ly wrote:
> I still need to inject OfferSettings for making the static ban cache -- is it fine if we keep piggybacking OfferSet on OfferSettings for now or do you mean I should inject both separately.
Ah, i missed the other input. I'm fine to keep it as-is.
> On Jan. 19, 2018, 10:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
> > Lines 23 (patched)
> > <https://reviews.apache.org/r/65233/diff/1/?file=1942317#file1942317line23>
> >
> > Up to you, but i'm not seeing a good reason to return `Iterable` over `Stream` here and below. It has the upside of no extra hoops for filtering and no need to protect from modification.
>
> Jordan Ly wrote:
> I agree that returning a Stream would be nicer, but I would like to keep the current interfaces below stable for this patch.
>
> I would like to do a larger refactor later (similar to your switch from Gauava to Java optionals) to target iterables and streams.
sgtm
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195838
-----------------------------------------------------------
On Jan. 19, 2018, 10:57 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 10:57 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195838
-----------------------------------------------------------
Ship it!
All nits, nice patch!
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
Line 66 (original), 65 (patched)
<https://reviews.apache.org/r/65233/#comment275166>
Inject `OfferSet` rather than `OfferSettings`.
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
Lines 180-182 (original), 177-180 (patched)
<https://reviews.apache.org/r/65233/#comment275168>
Revert. If there is no downside to chaining, i think it's much more readable.
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
Line 96 (original), 96 (patched)
<https://reviews.apache.org/r/65233/#comment275167>
Revert noop
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java
Line 91 (original), 91 (patched)
<https://reviews.apache.org/r/65233/#comment275169>
`offer_set_module` singular -> no custom splitter, no `List`.
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
Lines 9 (patched)
<https://reviews.apache.org/r/65233/#comment275170>
s/collection/set/. That distinction makes me comfortable leaving most of these methods without docs.
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
Lines 12 (patched)
<https://reviews.apache.org/r/65233/#comment275171>
Remove VisibleForTesting
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
Lines 23 (patched)
<https://reviews.apache.org/r/65233/#comment275175>
Up to you, but i'm not seeing a good reason to return `Iterable` over `Stream` here and below. It has the upside of no extra hoops for filtering and no need to protect from modification.
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
Lines 23-30 (patched)
<https://reviews.apache.org/r/65233/#comment275176>
How about this renaming:
`getAll()` -> `values()` (aligning with Collection terminology)
`getAll(...)` -> `getOrdered(...)`
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
Lines 26 (patched)
<https://reviews.apache.org/r/65233/#comment275177>
Please expand this doc a bit. What does context mean?
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java
Lines 78 (patched)
<https://reviews.apache.org/r/65233/#comment275172>
Omit comment, non-informative
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
Lines 72 (patched)
<https://reviews.apache.org/r/65233/#comment275173>
remove
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
Line 241 (original), 239 (patched)
<https://reviews.apache.org/r/65233/#comment275174>
Remove comment
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
Line 192 (original)
<https://reviews.apache.org/r/65233/#comment275178>
If i'm reading correctly, this test is still relevant with the default `OfferSetImpl`. Any reason we should remove it?
- Bill Farner
On Jan. 19, 2018, 10:11 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 10:11 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/2/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195855
-----------------------------------------------------------
Ship it!
Master (c4c55ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 1:40 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 1:40 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/4/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195874
-----------------------------------------------------------
Ship it!
Ship It!
- Stephan Erb
On Jan. 20, 2018, 2:31 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2018, 2:31 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/6/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195866
-----------------------------------------------------------
Ship it!
Master (c4c55ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 5:31 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 5:31 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/5/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195870
-----------------------------------------------------------
Ship it!
Master (c4c55ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 5:31 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 5:31 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/6/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
> On Jan. 20, 2018, 2:16 a.m., Bill Farner wrote:
> > docs/reference/scheduler-configuration.md
> > Lines 179 (patched)
> > <https://reviews.apache.org/r/65233/diff/5/?file=1943023#file1943023line179>
> >
> > Odd to see `OfferSetImpl` here. Should be a module, right?
Yep! Good catch, changed.
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195867
-----------------------------------------------------------
On Jan. 20, 2018, 1:31 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2018, 1:31 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/6/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195867
-----------------------------------------------------------
Ship it!
docs/reference/scheduler-configuration.md
Lines 179 (patched)
<https://reviews.apache.org/r/65233/#comment275206>
Odd to see `OfferSetImpl` here. Should be a module, right?
- Bill Farner
On Jan. 19, 2018, 5:31 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 5:31 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/5/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/
-----------------------------------------------------------
(Updated Jan. 20, 2018, 1:31 a.m.)
Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
Changes
-------
Re-add `offer_order` option for the default `OfferSetImpl`
Repository: aurora
Description (updated)
-------
The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
The following options will be removed from the last 0.19 to now:
```
-offer_order_modules
```
Diffs (updated)
-----
RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
Diff: https://reviews.apache.org/r/65233/diff/5/
Changes: https://reviews.apache.org/r/65233/diff/4-5/
Testing
-------
`./gradlew test`
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
Testing injection of a custom `OfferSet` onto a test cluster.
Thanks,
Jordan Ly
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/
-----------------------------------------------------------
(Updated Jan. 19, 2018, 9:40 p.m.)
Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
Changes
-------
Added `scheduler-configuration.md` entry, corrected `RELEASE_NOTES.md` entry.
Repository: aurora
Description
-------
The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
The following options will be removed from the last 0.19 to now:
```
-offer_order
-offer_order_modules
```
Diffs (updated)
-----
RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
Diff: https://reviews.apache.org/r/65233/diff/4/
Changes: https://reviews.apache.org/r/65233/diff/3-4/
Testing
-------
`./gradlew test`
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
Testing injection of a custom `OfferSet` onto a test cluster.
Thanks,
Jordan Ly
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195846
-----------------------------------------------------------
Ship it!
Master (c4c55ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 6:57 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 6:57 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195845
-----------------------------------------------------------
Ship it!
LGTM.
RELEASE-NOTES.md
Lines 15-17 (original), 15-17 (patched)
<https://reviews.apache.org/r/65233/#comment275187>
Update documentation for Scheduler configuration. Probably with an example.
RELEASE-NOTES.md
Lines 18-21 (patched)
<https://reviews.apache.org/r/65233/#comment275186>
This should move to the "Deprecations and removals" section.
- Santhosh Kumar Shanmugham
On Jan. 19, 2018, 10:57 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 10:57 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Stephan Erb <se...@apache.org>.
> On Jan. 19, 2018, 8:27 p.m., Stephan Erb wrote:
> > The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
> >
> > I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
>
> Jordan Ly wrote:
> Some context for the new interface:
>
> For performance, it is helpful to control the data structure we are holding offers in and how we can take advantage of it to narrow down the list of candidates to schedule on. For example, in the case of CPU ordering, if we call `getOrdered(TaskGroupKey, ResourceRequest)` -- since we know the size of the ResourceRequest and since we have access to the `offers` NavigableSet, we might use `subMap` to return only candidates that we know will fit thus reducing the number of offers we have to evaluate right away.
>
> Stephan Erb wrote:
> Yeah that makes sense, but would have also been possible without the set of recent scheduling refactorings (either by using the `NavigableSet` as-is or via something like https://github.com/wfarner/aurora/commit/f77d79a2d01c3b5b34f11b812d5dcff2789e0766 ). On top, this could even be done by default.
>
> If I am reading correctly between the lines of this RB here and https://reviews.apache.org/r/64954/ it sounds like you are moving towards a custom, heavy-weight scheduling logic. My question essentially boils down to: Do you go down the custom route because your usecase is so specific? Or is it just more involved doing it in the open (lack of reviews, increases turnaround times, ...)?
>
> I don't intend to block your patch, as it looks good. I just want to understand where this is heading and why.
>
> David McLaughlin wrote:
> Twitter has a 85/15 split between production (preferred) and preemtible tier tasks. The idea we're pursuing is to try and split the cluster logically into hosts that have preferred tasks and hosts that have preemtible tasks. Why is this useful? Well, the machines with preemptible tasks are effectively "empty" with preemption enabled, in terms of available slots that other jobs can grow into. It means if we commit 15% of our budget to the preemptible tier, we can also guarantee that we can support that % of growth in the cluster. This will enable larger instances, and temporary bursts, etc.
>
> Bin-packing alone doesn't solve this, because if you end up with every host 85% prod and 15% non-prod, and you are low on capacity (forcing preemption) only slots 15% the size of a host are available. This excludes many jobs from growing, and it forces us to put unnecessary "core limits" on jobs.
>
> I think this type of scheduling optimization is very specific to Twitter. As is the scale we're working at, which meant that score-based scheduling using just OfferSelector didn't work for us. But there are some neat optimizations we can make for just the condition we want, but it requires access to the underlying data structures (basically to support reverse iteration in a performant way). So all of this motivates this patch.
>
> Jordan Ly wrote:
> @StephanErb I now realize that you meant you were still utilizing the `offer_order` option and not the custom `offer_order_modules` one.
>
> I will re-add this in a follow-up patch since it can still co-exist with the default `OfferSetImpl`.
@David Thanks a lot for sharing! That is indeed a problem I have not encountered or considered before.
@Jordan Thanks!
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
On Jan. 20, 2018, 2:31 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2018, 2:31 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/6/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
> On Jan. 19, 2018, 7:27 p.m., Stephan Erb wrote:
> > The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
> >
> > I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
>
> Jordan Ly wrote:
> Some context for the new interface:
>
> For performance, it is helpful to control the data structure we are holding offers in and how we can take advantage of it to narrow down the list of candidates to schedule on. For example, in the case of CPU ordering, if we call `getOrdered(TaskGroupKey, ResourceRequest)` -- since we know the size of the ResourceRequest and since we have access to the `offers` NavigableSet, we might use `subMap` to return only candidates that we know will fit thus reducing the number of offers we have to evaluate right away.
>
> Stephan Erb wrote:
> Yeah that makes sense, but would have also been possible without the set of recent scheduling refactorings (either by using the `NavigableSet` as-is or via something like https://github.com/wfarner/aurora/commit/f77d79a2d01c3b5b34f11b812d5dcff2789e0766 ). On top, this could even be done by default.
>
> If I am reading correctly between the lines of this RB here and https://reviews.apache.org/r/64954/ it sounds like you are moving towards a custom, heavy-weight scheduling logic. My question essentially boils down to: Do you go down the custom route because your usecase is so specific? Or is it just more involved doing it in the open (lack of reviews, increases turnaround times, ...)?
>
> I don't intend to block your patch, as it looks good. I just want to understand where this is heading and why.
>
> David McLaughlin wrote:
> Twitter has a 85/15 split between production (preferred) and preemtible tier tasks. The idea we're pursuing is to try and split the cluster logically into hosts that have preferred tasks and hosts that have preemtible tasks. Why is this useful? Well, the machines with preemptible tasks are effectively "empty" with preemption enabled, in terms of available slots that other jobs can grow into. It means if we commit 15% of our budget to the preemptible tier, we can also guarantee that we can support that % of growth in the cluster. This will enable larger instances, and temporary bursts, etc.
>
> Bin-packing alone doesn't solve this, because if you end up with every host 85% prod and 15% non-prod, and you are low on capacity (forcing preemption) only slots 15% the size of a host are available. This excludes many jobs from growing, and it forces us to put unnecessary "core limits" on jobs.
>
> I think this type of scheduling optimization is very specific to Twitter. As is the scale we're working at, which meant that score-based scheduling using just OfferSelector didn't work for us. But there are some neat optimizations we can make for just the condition we want, but it requires access to the underlying data structures (basically to support reverse iteration in a performant way). So all of this motivates this patch.
@StephanErb I now realize that you meant you were still utilizing the `offer_order` option and not the custom `offer_order_modules` one.
I will re-add this in a follow-up patch since it can still co-exist with the default `OfferSetImpl`.
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
On Jan. 19, 2018, 9:40 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 9:40 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/4/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Stephan Erb <se...@apache.org>.
> On Jan. 19, 2018, 8:27 p.m., Stephan Erb wrote:
> > The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
> >
> > I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
>
> Jordan Ly wrote:
> Some context for the new interface:
>
> For performance, it is helpful to control the data structure we are holding offers in and how we can take advantage of it to narrow down the list of candidates to schedule on. For example, in the case of CPU ordering, if we call `getOrdered(TaskGroupKey, ResourceRequest)` -- since we know the size of the ResourceRequest and since we have access to the `offers` NavigableSet, we might use `subMap` to return only candidates that we know will fit thus reducing the number of offers we have to evaluate right away.
Yeah that makes sense, but would have also been possible without the set of recent scheduling refactorings (either by using the `NavigableSet` as-is or via something like https://github.com/wfarner/aurora/commit/f77d79a2d01c3b5b34f11b812d5dcff2789e0766 ). On top, this could even be done by default.
If I am reading correctly between the lines of this RB here and https://reviews.apache.org/r/64954/ it sounds like you are moving towards a custom, heavy-weight scheduling logic. My question essentially boils down to: Do you go down the custom route because your usecase is so specific? Or is it just more involved doing it in the open (lack of reviews, increases turnaround times, ...)?
I don't intend to block your patch, as it looks good. I just want to understand where this is heading and why.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
On Jan. 19, 2018, 10:40 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 10:40 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/4/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Jan. 19, 2018, 7:27 p.m., Stephan Erb wrote:
> > The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
> >
> > I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
>
> Jordan Ly wrote:
> Some context for the new interface:
>
> For performance, it is helpful to control the data structure we are holding offers in and how we can take advantage of it to narrow down the list of candidates to schedule on. For example, in the case of CPU ordering, if we call `getOrdered(TaskGroupKey, ResourceRequest)` -- since we know the size of the ResourceRequest and since we have access to the `offers` NavigableSet, we might use `subMap` to return only candidates that we know will fit thus reducing the number of offers we have to evaluate right away.
>
> Stephan Erb wrote:
> Yeah that makes sense, but would have also been possible without the set of recent scheduling refactorings (either by using the `NavigableSet` as-is or via something like https://github.com/wfarner/aurora/commit/f77d79a2d01c3b5b34f11b812d5dcff2789e0766 ). On top, this could even be done by default.
>
> If I am reading correctly between the lines of this RB here and https://reviews.apache.org/r/64954/ it sounds like you are moving towards a custom, heavy-weight scheduling logic. My question essentially boils down to: Do you go down the custom route because your usecase is so specific? Or is it just more involved doing it in the open (lack of reviews, increases turnaround times, ...)?
>
> I don't intend to block your patch, as it looks good. I just want to understand where this is heading and why.
Twitter has a 85/15 split between production (preferred) and preemtible tier tasks. The idea we're pursuing is to try and split the cluster logically into hosts that have preferred tasks and hosts that have preemtible tasks. Why is this useful? Well, the machines with preemptible tasks are effectively "empty" with preemption enabled, in terms of available slots that other jobs can grow into. It means if we commit 15% of our budget to the preemptible tier, we can also guarantee that we can support that % of growth in the cluster. This will enable larger instances, and temporary bursts, etc.
Bin-packing alone doesn't solve this, because if you end up with every host 85% prod and 15% non-prod, and you are low on capacity (forcing preemption) only slots 15% the size of a host are available. This excludes many jobs from growing, and it forces us to put unnecessary "core limits" on jobs.
I think this type of scheduling optimization is very specific to Twitter. As is the scale we're working at, which meant that score-based scheduling using just OfferSelector didn't work for us. But there are some neat optimizations we can make for just the condition we want, but it requires access to the underlying data structures (basically to support reverse iteration in a performant way). So all of this motivates this patch.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
On Jan. 19, 2018, 9:40 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 9:40 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/4/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
> On Jan. 19, 2018, 7:27 p.m., Stephan Erb wrote:
> > The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
> >
> > I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
Some context for the new interface:
For performance, it is helpful to control the data structure we are holding offers in and how we can take advantage of it to narrow down the list of candidates to schedule on. For example, in the case of CPU ordering, if we call `getOrdered(TaskGroupKey, ResourceRequest)` -- since we know the size of the ResourceRequest and since we have access to the `offers` NavigableSet, we might use `subMap` to return only candidates that we know will fit thus reducing the number of offers we have to evaluate right away.
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
On Jan. 19, 2018, 9:40 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 9:40 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> docs/reference/scheduler-configuration.md f697b6f1f5332ac9a9feade48317ecf5818e0c92
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/4/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195844
-----------------------------------------------------------
The patch itself looks fine! However, I have difficulties assesing it properly as I am missing a bit of context. What is the main customization that you aim to implement using the new interface?
I am also curious what is your take on the removed offer sorting logic. Does it still work for Twitter as intended originally? We are currently using it as well, so I would need to re-implement if it gets removed. (Main issue we have noticed so far was that the number of tasks per agent can shift drastically, sometimes even up to a point where the Thermos observer is completely unusable).
RELEASE-NOTES.md
Lines 18 (patched)
<https://reviews.apache.org/r/65233/#comment275185>
Should be moved to the `Deprecation and removals` section below.
- Stephan Erb
On Jan. 19, 2018, 7:57 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 7:57 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195843
-----------------------------------------------------------
Ship it!
Ship It!
- David McLaughlin
On Jan. 19, 2018, 6:57 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 6:57 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/
-----------------------------------------------------------
(Updated Jan. 19, 2018, 6:57 p.m.)
Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
Changes
-------
Address comments, added to RELEASE-NOTES.md
Repository: aurora
Description
-------
The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
The following options will be removed from the last 0.19 to now:
```
-offer_order
-offer_order_modules
```
Diffs (updated)
-----
RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
Diff: https://reviews.apache.org/r/65233/diff/3/
Changes: https://reviews.apache.org/r/65233/diff/2-3/
Testing
-------
`./gradlew test`
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
Testing injection of a custom `OfferSet` onto a test cluster.
Thanks,
Jordan Ly
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/
-----------------------------------------------------------
(Updated Jan. 19, 2018, 6:11 p.m.)
Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
Changes
-------
Fix JMH errors.
Repository: aurora
Description
-------
The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
The following options will be removed from the last 0.19 to now:
```
-offer_order
-offer_order_modules
```
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
Diff: https://reviews.apache.org/r/65233/diff/2/
Changes: https://reviews.apache.org/r/65233/diff/1-2/
Testing
-------
`./gradlew test`
`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
Testing injection of a custom `OfferSet` onto a test cluster.
Thanks,
Jordan Ly
Re: Review Request 65233: Allow for injection of custom OfferSets,
removed OfferOrder and OfferSelector
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65233/#review195785
-----------------------------------------------------------
Master (c4c55ff) is red with this patch.
./build-support/jenkins/build.sh
:processResources
:classes
:jar
:moreStartScripts
:startScripts
:distTar
:distZip
:assemble
:compileTestJavaNote: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/storage/durability/DurableStorageTest.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:processTestResources
:testClasses
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java:33:8: Unused import - com.google.inject.assistedinject.FactoryModuleBuilder. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java:1: Line does not match expected header line of '^\/\*\*$'. [RegexpHeader]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java:1: Line does not match expected header line of '^\/\*\*$'. [RegexpHeader]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java:21:8: Unused import - java.util.function.Predicate. [UnusedImports]
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
* 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 54s
32 actionable tasks: 26 executed, 6 up-to-date
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 19, 2018, 6:05 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 6:05 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> The goal of this patch is to provide a more reasonable abstraction for custom scheduling.
>
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were all created in order to provide more customization within the scheduling loop. However, they suffer from being leaky and too disparate. This patch hopes to combine all of those components into a single `OfferSet` which can be injected and used by HostOffers. This interface allows for custom scheduling logic to be co-located with custom data structures for `offers` as opposed to being passed around as different components.
>
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4
> src/main/java/org/apache/aurora/scheduler/config/CliOptions.java e4e535824adb86c98c6173a20f97a9a90b08483a
> src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 2ea7a01085b87c8ed6765537a8005e2349784ab0
> src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 8f9e33d81be9087821784a8a08079a1736d9cb63
> src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java de16c14c887d4225e0629b1580eb5e740798f1f7
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 7c99c309e0e97519719905f725bb9f6a59d2a7f5
> src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 1260ef19506acb8e8a937d4fd7b7152361bd3c40
> src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 1e36b2c3094e99f05aa4a4f098a48df2293b4320
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java 5f4f452cb133e2be73f881700a54ba4c8e60a35a
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java 4d36487f430483444314a8cefd133e7c766075de
> src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 6f5d55c5a586ac923b5f375d1935ec2049673ec0
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java d101a497896ba3b8ca7984a63f861494cbddbb35
> src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c
> src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 28224a5c587b235ccc86a286e796eeca66929232
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java e6b2b7465fb4fd432cda50d1e714dfc587764689
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java f84941edd96d974a6c2d7e087324bc8fbbac3b0a
>
>
> Diff: https://reviews.apache.org/r/65233/diff/1/
>
>
> Testing
> -------
>
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
>
> Testing injection of a custom `OfferSet` onto a test cluster.
>
>
> Thanks,
>
> Jordan Ly
>
>