You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by se...@apache.org on 2018/01/20 10:28:06 UTC

aurora git commit: Allow for injection of custom OfferSets, removed OfferOrder and OfferSelector

Repository: aurora
Updated Branches:
  refs/heads/master c4c55ff51 -> dbe713743


Allow for injection of custom OfferSets, removed OfferOrder and OfferSelector

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

Reviewed at https://reviews.apache.org/r/65233/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/dbe71374
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/dbe71374
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/dbe71374

Branch: refs/heads/master
Commit: dbe71374399d86d77baa35efbeca4fffa34f380e
Parents: c4c55ff
Author: Jordan Ly <jo...@gmail.com>
Authored: Sat Jan 20 11:27:54 2018 +0100
Committer: Stephan Erb <se...@apache.org>
Committed: Sat Jan 20 11:27:54 2018 +0100

----------------------------------------------------------------------
 RELEASE-NOTES.md                                |  8 ++-
 docs/reference/scheduler-configuration.md       |  2 +
 .../aurora/benchmark/SchedulingBenchmarks.java  |  5 +-
 .../aurora/scheduler/config/CliOptions.java     |  2 -
 .../aurora/scheduler/offers/HostOffers.java     | 24 +++----
 .../aurora/scheduler/offers/OfferManager.java   |  2 +-
 .../scheduler/offers/OfferManagerModule.java    | 32 +++++----
 .../scheduler/offers/OfferOrderBuilder.java     |  7 +-
 .../aurora/scheduler/offers/OfferSet.java       | 41 ++++++++++++
 .../aurora/scheduler/offers/OfferSetImpl.java   | 70 ++++++++++++++++++++
 .../aurora/scheduler/offers/OfferSettings.java  | 30 ++-------
 .../scheduling/FirstFitOfferSelector.java       | 30 ---------
 .../scheduling/FirstFitOfferSelectorModule.java | 26 --------
 .../scheduler/scheduling/OfferSelector.java     | 36 ----------
 .../scheduler/scheduling/TaskAssignerImpl.java  | 18 ++---
 .../scheduling/TaskAssignerImplModule.java      | 33 +--------
 .../scheduler/config/CommandLineTest.java       | 10 +--
 .../scheduler/offers/OfferManagerImplTest.java  |  6 +-
 .../scheduling/FirstFitOfferSelectorTest.java   | 63 ------------------
 .../scheduling/TaskAssignerImplTest.java        |  6 +-
 20 files changed, 177 insertions(+), 274 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/RELEASE-NOTES.md
----------------------------------------------------------------------
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 3053e54..983088e 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -12,15 +12,17 @@
   a production cluster. For that reason, the functionality is behind a new flag `-partition_aware`
   that is disabled by default. When Mesos support is improved and the new behavior is vetted in
   production clusters, we'll enable this by default.
-- Added the ability to "score" offers for a given scheduling assignment via the `OfferSelector`
-  interface. The default implementation is first fit, but cluster operators can inject a custom
-  scoring algorithm through the `-offer_selector_modules` flag.
+- Added the ability to inject custom offer holding and scheduling logic via the `-offer_set_module`
+  scheduler flag. To take advantage of this feature, you will need to implement the `OfferSet`
+  interface.
 
 ### Deprecations and removals:
 
 - Removed the ability to recover from SQL-based backups and snapshots.  An 0.20.0 scheduler
   will not be able to recover backups or replicated log data created prior to 0.19.0.
 - Removed task level resource fields (`numCpus`, `ramMb`, `diskMb`, `requestedPorts`).
+- Removed the `-offer_order_modules` scheduler flag related to custom injectable offer orderings,
+  since this will now be subsumed under custom `OfferSet` implementations (see the comment above):
 
 0.19.0
 ======

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/docs/reference/scheduler-configuration.md
----------------------------------------------------------------------
diff --git a/docs/reference/scheduler-configuration.md b/docs/reference/scheduler-configuration.md
index f697b6f..a659cfa 100644
--- a/docs/reference/scheduler-configuration.md
+++ b/docs/reference/scheduler-configuration.md
@@ -176,6 +176,8 @@ Optional flags:
 	Maximum amount of random jitter to add to the offer hold time window.
 -offer_reservation_duration (default (3, mins))
 	Time to reserve a agent's offers while trying to satisfy a task preempting another.
+-offer_set_module (default [class org.apache.aurora.scheduler.offers.OfferSetModule])
+  Guice module for replacing offer holding and scheduling logic.
 -partition_aware (default false)
   Whether or not to integrate with the partition-aware Mesos capabilities.
 -populate_discovery_info (default false)

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
----------------------------------------------------------------------
diff --git a/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java b/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
index f44cb61..54b6ed9 100644
--- a/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
+++ b/src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
@@ -65,6 +65,8 @@ import org.apache.aurora.scheduler.offers.OfferManager;
 import org.apache.aurora.scheduler.offers.OfferManagerImpl;
 import org.apache.aurora.scheduler.offers.OfferManagerModule;
 import org.apache.aurora.scheduler.offers.OfferOrder;
+import org.apache.aurora.scheduler.offers.OfferOrderBuilder;
+import org.apache.aurora.scheduler.offers.OfferSetImpl;
 import org.apache.aurora.scheduler.offers.OfferSettings;
 import org.apache.aurora.scheduler.preemptor.BiCache;
 import org.apache.aurora.scheduler.preemptor.ClusterStateImpl;
@@ -153,7 +155,8 @@ public class SchedulingBenchmarks {
               bind(OfferManagerImpl.class).in(Singleton.class);
               bind(OfferSettings.class).toInstance(
                   new OfferSettings(NO_DELAY,
-                      ImmutableList.of(OfferOrder.RANDOM),
+                      new OfferSetImpl(
+                          OfferOrderBuilder.create(ImmutableList.of(OfferOrder.RANDOM))),
                       Amount.of(Long.MAX_VALUE, Time.SECONDS),
                       Long.MAX_VALUE,
                       new FakeTicker()));

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/config/CliOptions.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/config/CliOptions.java b/src/main/java/org/apache/aurora/scheduler/config/CliOptions.java
index e4e5358..a2fb039 100644
--- a/src/main/java/org/apache/aurora/scheduler/config/CliOptions.java
+++ b/src/main/java/org/apache/aurora/scheduler/config/CliOptions.java
@@ -42,7 +42,6 @@ import org.apache.aurora.scheduler.pruning.PruningModule;
 import org.apache.aurora.scheduler.reconciliation.ReconciliationModule;
 import org.apache.aurora.scheduler.resources.ResourceSettings;
 import org.apache.aurora.scheduler.scheduling.SchedulingModule;
-import org.apache.aurora.scheduler.scheduling.TaskAssignerImplModule;
 import org.apache.aurora.scheduler.sla.SlaModule;
 import org.apache.aurora.scheduler.state.StateModule;
 import org.apache.aurora.scheduler.stats.AsyncStatsModule;
@@ -87,7 +86,6 @@ public class CliOptions {
   public final StatsModule.Options stats = new StatsModule.Options();
   public final CronModule.Options cron = new CronModule.Options();
   public final ResourceSettings resourceSettings = new ResourceSettings();
-  public final TaskAssignerImplModule.Options taskAssigner = new TaskAssignerImplModule.Options();
   final List<Object> custom;
 
   public CliOptions() {

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java b/src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
index 2ea7a01..6011d96 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
@@ -16,7 +16,6 @@ package org.apache.aurora.scheduler.offers;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.atomic.AtomicLong;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -43,7 +42,7 @@ import static java.util.Objects.requireNonNull;
  * reason about the different indices used and their consistency.
  */
 class HostOffers {
-  private final Set<HostOffer> offers;
+  private final OfferSet offers;
 
   private final Map<Protos.OfferID, HostOffer> offersById = Maps.newHashMap();
   private final Map<Protos.AgentID, HostOffer> offersBySlave = Maps.newHashMap();
@@ -63,15 +62,13 @@ class HostOffers {
   HostOffers(StatsProvider statsProvider,
              OfferSettings offerSettings,
              SchedulingFilter schedulingFilter) {
-    this.offers = new ConcurrentSkipListSet<>(offerSettings.getOrdering());
+    this.offers = offerSettings.getOfferSet();
     this.staticallyBannedOffers = offerSettings
         .getStaticBanCacheBuilder()
         .build();
     this.schedulingFilter = requireNonNull(schedulingFilter);
 
-    // Potential gotcha - since this is a ConcurrentSkipListSet, size() is more expensive.
-    // Could track this separately if it turns out to pose problems.
-    statsProvider.exportSize(OfferManagerImpl.OUTSTANDING_OFFERS, offers);
+    statsProvider.makeGauge(OfferManagerImpl.OUTSTANDING_OFFERS, offers::size);
     statsProvider.makeGauge(OfferManagerImpl.STATICALLY_BANNED_OFFERS,
         staticallyBannedOffers::size);
     statsProvider.makeGauge(OfferManagerImpl.STATICALLY_BANNED_OFFERS_HIT_RATE,
@@ -149,8 +146,8 @@ class HostOffers {
    * @return The offers currently known by the scheduler.
    */
   synchronized Iterable<HostOffer> getOffers() {
-    return FluentIterable.from(offers)
-        .filter(o -> !globallyBannedOffers.contains(o.getOffer().getId()))
+    return FluentIterable.from(offers.values())
+        .filter(offer -> !globallyBannedOffers.contains(offer.getOffer().getId()))
         .toSet();
   }
 
@@ -175,11 +172,12 @@ class HostOffers {
   synchronized Iterable<HostOffer> getAllMatching(TaskGroupKey groupKey,
                                                   ResourceRequest resourceRequest) {
 
-    return Iterables.unmodifiableIterable(FluentIterable.from(offers)
-        .filter(o -> !isGloballyBanned(o))
-        .filter(o -> !isStaticallyBanned(o, groupKey))
-        .filter(HostOffer::hasCpuAndMem)
-        .filter(o -> !isVetoed(o, resourceRequest, Optional.of(groupKey))));
+    return Iterables.unmodifiableIterable(
+        FluentIterable.from(offers.getOrdered(groupKey, resourceRequest))
+            .filter(o -> !isGloballyBanned(o))
+            .filter(o -> !isStaticallyBanned(o, groupKey))
+            .filter(HostOffer::hasCpuAndMem)
+            .filter(o -> !isVetoed(o, resourceRequest, Optional.of(groupKey))));
   }
 
   private synchronized boolean isGloballyBanned(HostOffer offer) {

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
index 8f9e33d..5d16fef 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java
@@ -91,7 +91,7 @@ public interface OfferManager extends EventSubscriber {
    *
    * @param groupKey The {@link TaskGroupKey} of the task in the {@link ResourceRequest}.
    * @param resourceRequest The request that the offer should satisfy.
-   * @return An option containing the offer for the slave ID if it fits.
+   * @return An Iterable containing all offers that satisfy the {@link ResourceRequest}.
    */
   Iterable<HostOffer> getAllMatching(TaskGroupKey groupKey, ResourceRequest resourceRequest);
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java
index de16c14..3ea6e5c 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java
@@ -27,7 +27,6 @@ import com.google.common.base.Ticker;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Ordering;
 import com.google.inject.AbstractModule;
-import com.google.inject.Module;
 import com.google.inject.PrivateModule;
 import com.google.inject.Provides;
 import com.google.inject.TypeLiteral;
@@ -96,11 +95,10 @@ public class OfferManagerModule extends AbstractModule {
         splitter = CommaSplitter.class)
     public List<OfferOrder> offerOrder = ImmutableList.of(OfferOrder.RANDOM);
 
-    @Parameter(names = "-offer_order_modules",
-        description = "Custom Guice module to provide an offer ordering.",
-        splitter = CommaSplitter.class)
+    @Parameter(names = "-offer_set_module",
+        description = "Custom Guice module to provide a custom OfferSet.")
     @SuppressWarnings("rawtypes")
-    public List<Class> offerOrderModules = ImmutableList.of(OfferOrderModule.class);
+    public Class offerSetModule = OfferSetModule.class;
 
     @Parameter(names = "-offer_static_ban_cache_max_size",
         validateValueWith = NotNegativeNumber.class,
@@ -118,17 +116,25 @@ public class OfferManagerModule extends AbstractModule {
   @Target({ FIELD, PARAMETER, METHOD }) @Retention(RUNTIME)
   public @interface UnavailabilityThreshold { }
 
-  public static class OfferOrderModule extends AbstractModule {
+  public static class OfferSetModule extends AbstractModule {
     private final CliOptions options;
 
-    public OfferOrderModule(CliOptions options) {
+    public OfferSetModule(CliOptions options) {
       this.options = options;
     }
 
     @Override
     protected void configure() {
-      bind(new TypeLiteral<Ordering<HostOffer>>() { })
-          .toInstance(OfferOrderBuilder.create(options.offer.offerOrder));
+      install(new PrivateModule() {
+        @Override
+        protected void configure() {
+          bind(new TypeLiteral<Ordering<HostOffer>>() { })
+              .toInstance(OfferOrderBuilder.create(options.offer.offerOrder));
+          bind(OfferSetImpl.class).in(Singleton.class);
+          bind(OfferSet.class).to(OfferSetImpl.class);
+          expose(OfferSet.class);
+        }
+      });
     }
   }
 
@@ -154,9 +160,7 @@ public class OfferManagerModule extends AbstractModule {
       }
     }
 
-    for (Module module: MoreModules.instantiateAll(options.offerOrderModules, cliOptions)) {
-      install(module);
-    }
+    install(MoreModules.instantiate(options.offerSetModule, cliOptions));
 
     bind(new TypeLiteral<Amount<Long, Time>>() { })
         .annotatedWith(UnavailabilityThreshold.class)
@@ -186,7 +190,7 @@ public class OfferManagerModule extends AbstractModule {
 
   @Provides
   @Singleton
-  OfferSettings provideOfferSettings(Ordering<HostOffer> offerOrdering) {
+  OfferSettings provideOfferSettings(OfferSet offerSet) {
     // We have a dual eviction strategy for the static ban cache in OfferManager that is based on
     // both maximum size of the cache and the length an offer is valid. We do this in order to
     // satisfy requirements in both single- and multi-framework environments. If offers are held for
@@ -205,7 +209,7 @@ public class OfferManagerModule extends AbstractModule {
 
     return new OfferSettings(
         cliOptions.offer.offerFilterDuration,
-        offerOrdering,
+        offerSet,
         Amount.of(maxOfferHoldTime, Time.SECONDS),
         cliOptions.offer.offerStaticBanCacheMaxSize,
         Ticker.systemTicker());

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java
index 1260ef1..fd118c1 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java
@@ -16,6 +16,7 @@ package org.apache.aurora.scheduler.offers;
 import java.time.Instant;
 import java.util.List;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Ordering;
 
 import org.apache.aurora.scheduler.resources.ResourceType;
@@ -31,7 +32,8 @@ import static org.apache.aurora.scheduler.resources.ResourceManager.getRevocable
 /**
  * Utility class for creating compounded offer orders based on some combination of offer ordering.
  */
-final class OfferOrderBuilder {
+@VisibleForTesting
+public final class OfferOrderBuilder {
   private OfferOrderBuilder() {
 
   }
@@ -112,7 +114,8 @@ final class OfferOrderBuilder {
    * @param order The list of offer orders. They will be compounded in the list order.
    * @return A HostOffer ordering.
    */
-  static Ordering<HostOffer> create(List<OfferOrder> order) {
+  @VisibleForTesting
+  public static Ordering<HostOffer> create(List<OfferOrder> order) {
     return create(BASE_COMPARATOR, order);
   }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
new file mode 100644
index 0000000..089837b
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
@@ -0,0 +1,41 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.offers;
+
+import org.apache.aurora.scheduler.base.TaskGroupKey;
+import org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
+
+/**
+ * A set that holds all offers within the scheduler, used internally by HostOffers. This
+ * interface is injectable via the '-offer_set_module' flag.
+ */
+public interface OfferSet {
+
+  void add(HostOffer offer);
+
+  void remove(HostOffer removed);
+
+  int size();
+
+  void clear();
+
+  Iterable<HostOffer> values();
+
+  /**
+   * Get an ordered stream of offers to consider given a {@link TaskGroupKey} and
+   * {@link ResourceRequest} as context. For example, an implementation may return a different
+   * ordering depending on the name of the task, or if the task is revocable.
+   */
+  Iterable<HostOffer> getOrdered(TaskGroupKey groupKey, ResourceRequest resourceRequest);
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java
new file mode 100644
index 0000000..2ce64b2
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java
@@ -0,0 +1,70 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.offers;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentSkipListSet;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Ordering;
+import com.google.inject.Inject;
+
+import org.apache.aurora.scheduler.base.TaskGroupKey;
+import org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
+
+/**
+ * Default implementation for OfferSet. Backed by a ConcurrentSkipListSet.
+ */
+@VisibleForTesting
+public class OfferSetImpl implements OfferSet {
+
+  private final Set<HostOffer> offers;
+
+  @Inject
+  public OfferSetImpl(Ordering<HostOffer> ordering) {
+    offers = new ConcurrentSkipListSet<>(ordering);
+  }
+
+  @Override
+  public void add(HostOffer offer) {
+    offers.add(offer);
+  }
+
+  @Override
+  public void remove(HostOffer removed) {
+    offers.remove(removed);
+  }
+
+  @Override
+  public int size() {
+    // Potential gotcha - since this is a ConcurrentSkipListSet, size() is more expensive.
+    // Could track this separately if it turns out to pose problems.
+    return offers.size();
+  }
+
+  @Override
+  public void clear() {
+    offers.clear();
+  }
+
+  @Override
+  public Iterable<HostOffer> values() {
+    return offers;
+  }
+
+  @Override
+  public Iterable<HostOffer> getOrdered(TaskGroupKey groupKey, ResourceRequest resourceRequest) {
+    return offers;
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java b/src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
index 1e36b2c..f14564f 100644
--- a/src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
+++ b/src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java
@@ -13,12 +13,9 @@
  */
 package org.apache.aurora.scheduler.offers;
 
-import java.util.List;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Ticker;
 import com.google.common.cache.CacheBuilder;
-import com.google.common.collect.Ordering;
 
 import org.apache.aurora.common.quantity.Amount;
 import org.apache.aurora.common.quantity.Time;
@@ -32,31 +29,18 @@ import static java.util.Objects.requireNonNull;
 public class OfferSettings {
 
   private final Amount<Long, Time> filterDuration;
-  private final Ordering<HostOffer> ordering;
+  private final OfferSet offerSet;
   private final CacheBuilder<Object, Object> staticBanCacheBuilder;
 
   @VisibleForTesting
   public OfferSettings(Amount<Long, Time> filterDuration,
-                       List<OfferOrder> ordering,
+                       OfferSet offerSet,
                        Amount<Long, Time> maxHoldTime,
                        long staticBanCacheMaxSize,
-                       Ticker staticBanCacheTicker) {
-
-    this(filterDuration,
-        OfferOrderBuilder.create(ordering),
-        maxHoldTime,
-        staticBanCacheMaxSize,
-        staticBanCacheTicker);
-  }
-
-  OfferSettings(Amount<Long, Time> filterDuration,
-                Ordering<HostOffer> ordering,
-                Amount<Long, Time> maxHoldTime,
-                long staticBanCacheMaxSize,
-                Ticker staticBanTicker) {
+                       Ticker staticBanTicker) {
 
     this.filterDuration = requireNonNull(filterDuration);
-    this.ordering = requireNonNull(ordering);
+    this.offerSet = requireNonNull(offerSet);
     this.staticBanCacheBuilder = CacheBuilder.newBuilder()
         .expireAfterWrite(maxHoldTime.as(Time.SECONDS), Time.SECONDS.getTimeUnit())
         .maximumSize(staticBanCacheMaxSize)
@@ -72,10 +56,10 @@ public class OfferSettings {
   }
 
   /**
-   * The ordering to use when fetching offers from OfferManager.
+   * The factory to create the {@link OfferSet} implementation.
    */
-  Ordering<HostOffer> getOrdering() {
-    return ordering;
+  OfferSet getOfferSet() {
+    return offerSet;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java b/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java
deleted file mode 100644
index 5f4f452..0000000
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java
+++ /dev/null
@@ -1,30 +0,0 @@
-/**
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.aurora.scheduler.scheduling;
-
-import java.util.Optional;
-
-import com.google.common.collect.Iterables;
-
-import org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
-import org.apache.aurora.scheduler.offers.HostOffer;
-
-public class FirstFitOfferSelector implements OfferSelector {
-
-  @Override
-  public Optional<HostOffer> select(Iterable<HostOffer> offers, ResourceRequest resourceRequest) {
-
-    return Optional.ofNullable(Iterables.getFirst(offers, null));
-  }
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java b/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java
deleted file mode 100644
index 4d36487..0000000
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java
+++ /dev/null
@@ -1,26 +0,0 @@
-/**
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.aurora.scheduler.scheduling;
-
-import com.google.inject.AbstractModule;
-import com.google.inject.Singleton;
-
-public class FirstFitOfferSelectorModule extends AbstractModule {
-
-  @Override
-  protected void configure() {
-    bind(OfferSelector.class).to(FirstFitOfferSelector.class);
-    bind(FirstFitOfferSelector.class).in(Singleton.class);
-  }
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java b/src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java
deleted file mode 100644
index 6f5d55c..0000000
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java
+++ /dev/null
@@ -1,36 +0,0 @@
-/**
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.aurora.scheduler.scheduling;
-
-import java.util.Optional;
-
-import org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
-import org.apache.aurora.scheduler.offers.HostOffer;
-
-/**
- * Injected into {@link TaskAssignerImpl}, this class scores the offers available and returns an
- * option containing the offer to use.
- */
-public interface OfferSelector {
-
-  /**
-   * Score offers that fit within the given {@link ResourceRequest} and return an option containing
-   * the offer to use for assignment.
-   *
-   * @param offers A stream of offers that match the given {@link ResourceRequest}.
-   * @param resourceRequest The {@link ResourceRequest} for the task to assign.
-   * @return An {@link Optional} containing the offer to use.
-   */
-  Optional<HostOffer> select(Iterable<HostOffer> offers, ResourceRequest resourceRequest);
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
index ec416cc..916908b 100644
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java
@@ -66,7 +66,6 @@ public class TaskAssignerImpl implements TaskAssigner {
   private final MesosTaskFactory taskFactory;
   private final OfferManager offerManager;
   private final UpdateAgentReserver updateAgentReserver;
-  private final OfferSelector offerSelector;
 
   @Inject
   public TaskAssignerImpl(
@@ -74,15 +73,13 @@ public class TaskAssignerImpl implements TaskAssigner {
       MesosTaskFactory taskFactory,
       OfferManager offerManager,
       UpdateAgentReserver updateAgentReserver,
-      StatsProvider statsProvider,
-      OfferSelector offerSelector) {
+      StatsProvider statsProvider) {
 
     this.stateManager = requireNonNull(stateManager);
     this.taskFactory = requireNonNull(taskFactory);
     this.offerManager = requireNonNull(offerManager);
     this.launchFailures = statsProvider.makeCounter(ASSIGNER_LAUNCH_FAILURES);
     this.updateAgentReserver = requireNonNull(updateAgentReserver);
-    this.offerSelector = requireNonNull(offerSelector);
   }
 
   @VisibleForTesting
@@ -232,21 +229,18 @@ public class TaskAssignerImpl implements TaskAssigner {
         chosenOffer = reservation.getOffer();
       } else {
         // Get all offers that will satisfy the given ResourceRequest and that are not reserved
-        // for updates or preemption
+        // for updates or preemption.
         Iterable<HostOffer> matchingOffers = Iterables.filter(
             offerManager.getAllMatching(groupKey, resourceRequest),
             o -> !matchesByOffer.containsKey(o.getOffer().getId().getValue())
                 && !isAgentReserved(o, groupKey, preemptionReservations));
 
-        // Determine which is the optimal offer to select for the given request
-        chosenOffer = offerSelector.select(matchingOffers, resourceRequest);
+        chosenOffer = Optional.ofNullable(Iterables.getFirst(matchingOffers, null));
       }
 
-      chosenOffer.ifPresent(hostOffer -> {
-        matchesByOffer.put(
-            hostOffer.getOffer().getId().getValue(),
-            new SchedulingMatch(task, hostOffer));
-      });
+      chosenOffer.ifPresent(hostOffer -> matchesByOffer.put(
+          hostOffer.getOffer().getId().getValue(),
+          new SchedulingMatch(task, hostOffer)));
     });
 
     return matchesByOffer.values();

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
index d101a49..e20c6fe 100644
--- a/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
@@ -13,48 +13,17 @@
  */
 package org.apache.aurora.scheduler.scheduling;
 
-import java.util.List;
 import javax.inject.Singleton;
 
-import com.beust.jcommander.Parameter;
-import com.beust.jcommander.Parameters;
-import com.google.common.collect.ImmutableList;
 import com.google.inject.AbstractModule;
-import com.google.inject.Module;
-
-import org.apache.aurora.scheduler.app.MoreModules;
-import org.apache.aurora.scheduler.config.CliOptions;
-import org.apache.aurora.scheduler.config.splitters.CommaSplitter;
 
 /**
- * The default TaskAssigner implementation that allows the injection of custom offer
- * selecting modules via the '-offer_selector_modules' flag.
+ * The default TaskAssigner implementation.
  */
 public class TaskAssignerImplModule extends AbstractModule {
 
-  @Parameters(separators = "=")
-  public static class Options {
-    @Parameter(names = "-offer_selector_modules",
-        description = "Guice module for customizing the TaskAssignerImpl's OfferSelector.",
-        splitter = CommaSplitter.class)
-    @SuppressWarnings("rawtypes")
-    public List<Class> offerSelectorModules =
-        ImmutableList.of(FirstFitOfferSelectorModule.class);
-  }
-
-  private final CliOptions cliOptions;
-
-  public TaskAssignerImplModule(CliOptions cliOptions) {
-    this.cliOptions = cliOptions;
-  }
-
   @Override
   protected void configure() {
-    Options options = cliOptions.taskAssigner;
-    for (Module module : MoreModules.instantiateAll(options.offerSelectorModules, cliOptions)) {
-      install(module);
-    }
-
     bind(TaskAssigner.class).to(TaskAssignerImpl.class);
     bind(TaskAssignerImpl.class).in(Singleton.class);
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java b/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
index c84fad3..0a16d3c 100644
--- a/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
@@ -123,7 +123,7 @@ public class CommandLineTest {
     expected.offer.offerFilterDuration = TEST_TIME;
     expected.offer.unavailabilityThreshold = TEST_TIME;
     expected.offer.offerOrder = ImmutableList.of(OfferOrder.CPU, OfferOrder.DISK);
-    expected.offer.offerOrderModules = ImmutableList.of(NoopModule.class);
+    expected.offer.offerSetModule = NoopModule.class;
     expected.executor.customExecutorConfig = tempFile;
     expected.executor.thermosExecutorPath = "testing";
     expected.executor.thermosExecutorResources = ImmutableList.of("testing");
@@ -160,7 +160,6 @@ public class CommandLineTest {
     expected.scheduling.reservationDuration = TEST_TIME;
     expected.scheduling.schedulingMaxBatchSize = 42;
     expected.scheduling.maxTasksPerScheduleAttempt = 42;
-    expected.taskAssigner.offerSelectorModules = ImmutableList.of(NoopModule.class);
     expected.async.asyncWorkerThreads = 42;
     expected.zk.inProcess = true;
     expected.zk.zkEndpoints = ImmutableList.of(InetSocketAddress.createUnresolved("testing", 42));
@@ -269,7 +268,7 @@ public class CommandLineTest {
         "-offer_filter_duration=42days",
         "-unavailability_threshold=42days",
         "-offer_order=CPU,DISK",
-        "-offer_order_modules=org.apache.aurora.scheduler.config.CommandLineTest$NoopModule",
+        "-offer_set_module=org.apache.aurora.scheduler.config.CommandLineTest$NoopModule",
         "-offer_static_ban_cache_max_size=42",
         "-custom_executor_config=" + tempFile.getAbsolutePath(),
         "-thermos_executor_path=testing",
@@ -306,7 +305,6 @@ public class CommandLineTest {
         "-offer_reservation_duration=42days",
         "-scheduling_max_batch_size=42",
         "-max_tasks_per_schedule_attempt=42",
-        "-offer_selector_modules=org.apache.aurora.scheduler.config.CommandLineTest$NoopModule",
         "-async_worker_threads=42",
         "-zk_in_proc=true",
         "-zk_endpoints=testing:42",
@@ -403,12 +401,10 @@ public class CommandLineTest {
     expected.main.serversetPath = "testing";
     expected.zk.zkEndpoints = ImmutableList.of(InetSocketAddress.createUnresolved("testing", 42));
     expected.offer.offerOrder = ImmutableList.of();
-    expected.offer.offerOrderModules = ImmutableList.of();
     expected.executor.thermosExecutorResources = ImmutableList.of();
     expected.executor.globalContainerMounts = ImmutableList.of();
     expected.app.allowedContainerTypes = ImmutableList.of();
     expected.app.defaultDockerParameters = ImmutableList.of();
-    expected.taskAssigner.offerSelectorModules = ImmutableList.of();
     expected.state.taskAssignerModules = ImmutableList.of();
     expected.aop.methodInterceptorModules = ImmutableList.of();
     expected.httpSecurity.shiroRealmModule = ImmutableList.of();
@@ -423,12 +419,10 @@ public class CommandLineTest {
             "-serverset_path=testing",
             "-zk_endpoints=testing:42",
             "-offer_order=",
-            "-offer_order_modules=",
             "-thermos_executor_resources=",
             "-global_container_mounts=",
             "-allowed_container_types=",
             "-default_docker_parameters=",
-            "-offer_selector_modules=",
             "-task_assigner_modules=",
             "-thrift_method_interceptor_modules=",
             "-shiro_realm_modules=",

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java b/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
index 28224a5..02d74db 100644
--- a/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java
@@ -128,7 +128,7 @@ public class OfferManagerImplTest extends EasyMockTest {
     driver = createMock(Driver.class);
     OfferSettings offerSettings = new OfferSettings(
         Amount.of(OFFER_FILTER_SECONDS, Time.SECONDS),
-        ImmutableList.of(OfferOrder.RANDOM),
+        new OfferSetImpl(OfferOrderBuilder.create(ImmutableList.of(OfferOrder.RANDOM))),
         RETURN_DELAY,
         Long.MAX_VALUE,
         FAKE_TICKER
@@ -386,7 +386,7 @@ public class OfferManagerImplTest extends EasyMockTest {
     OfferSettings settings =
         new OfferSettings(
             Amount.of(OFFER_FILTER_SECONDS, Time.SECONDS),
-            order,
+            new OfferSetImpl(OfferOrderBuilder.create(order)),
             RETURN_DELAY,
             Long.MAX_VALUE,
             FAKE_TICKER);
@@ -561,7 +561,7 @@ public class OfferManagerImplTest extends EasyMockTest {
   public void testDelayedOfferReturn() {
     OfferSettings settings = new OfferSettings(
         Amount.of(OFFER_FILTER_SECONDS, Time.SECONDS),
-        ImmutableList.of(OfferOrder.RANDOM),
+        new OfferSetImpl(OfferOrderBuilder.create(ImmutableList.of(OfferOrder.RANDOM))),
         RETURN_DELAY,
         Long.MAX_VALUE,
         FAKE_TICKER);

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java b/src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java
deleted file mode 100644
index e6b2b74..0000000
--- a/src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java
+++ /dev/null
@@ -1,63 +0,0 @@
-/**
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.aurora.scheduler.scheduling;
-
-import com.google.common.collect.ImmutableList;
-
-import org.apache.aurora.common.testing.easymock.EasyMockTest;
-import org.apache.aurora.scheduler.base.TaskTestUtil;
-import org.apache.aurora.scheduler.offers.HostOffer;
-import org.apache.aurora.scheduler.storage.entities.IAssignedTask;
-import org.junit.Before;
-import org.junit.Test;
-
-import static org.apache.aurora.scheduler.base.TaskTestUtil.JOB;
-import static org.apache.aurora.scheduler.base.TaskTestUtil.makeTask;
-import static org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-
-public class FirstFitOfferSelectorTest extends EasyMockTest {
-
-  private static final IAssignedTask TASK = makeTask("id", JOB).getAssignedTask();
-  private static final ResourceRequest EMPTY_REQUEST =
-      TaskTestUtil.toResourceRequest(TASK.getTask());
-
-  private OfferSelector firstFitOfferSelector;
-
-  @Before
-  public void setUp() {
-    firstFitOfferSelector = new FirstFitOfferSelector();
-  }
-
-  @Test
-  public void testNoOffers() {
-    Iterable<HostOffer> offers = ImmutableList.of();
-
-    control.replay();
-
-    assertFalse(firstFitOfferSelector.select(offers, EMPTY_REQUEST).isPresent());
-  }
-
-  @Test
-  public void testReturnFirstOffer() {
-    HostOffer offerA = createMock(HostOffer.class);
-    HostOffer offerB = createMock(HostOffer.class);
-    Iterable<HostOffer> offers = ImmutableList.of(offerA, offerB);
-
-    control.replay();
-
-    assertEquals(offerA, firstFitOfferSelector.select(offers, EMPTY_REQUEST).get());
-  }
-}

http://git-wip-us.apache.org/repos/asf/aurora/blob/dbe71374/src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java b/src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java
index f84941e..533bb44 100644
--- a/src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java
@@ -120,16 +120,12 @@ public class TaskAssignerImplTest extends EasyMockTest {
     offerManager = createMock(OfferManager.class);
     updateAgentReserver = createMock(UpdateAgentReserver.class);
     statsProvider = new FakeStatsProvider();
-    // TODO(jly): FirstFitOfferSelector returns the first offer which is what we want for testing,
-    // but if its implementation becomes more complex we may need to replace it with a fake.
-    OfferSelector offerSelector = new FirstFitOfferSelector();
     assigner = new TaskAssignerImpl(
         stateManager,
         taskFactory,
         offerManager,
         updateAgentReserver,
-        statsProvider,
-        offerSelector);
+        statsProvider);
     aggregate = empty();
     resourceRequest = ResourceRequest.fromTask(
         TASK.getTask(),