From 98812769a5adefaa7f2b17da4e3e8759c529b59d Mon Sep 17 00:00:00 2001 From: kohsuke Date: Thu, 23 Apr 2009 18:06:15 +0000 Subject: [PATCH] extracted the scheduling strategy into a separate class, which we intend to make pluggable git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@17435 71c3de6d-444a-0410-be80-ed276b4c234a --- core/src/main/java/hudson/model/Hudson.java | 2 +- .../main/java/hudson/model/LoadBalancer.java | 138 ++++++++++++++++++ core/src/main/java/hudson/model/Queue.java | 75 ++-------- 3 files changed, 152 insertions(+), 63 deletions(-) create mode 100644 core/src/main/java/hudson/model/LoadBalancer.java diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java index f2bcf88df6..7ad8565120 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -495,7 +495,7 @@ public final class Hudson extends Node implements ItemGroup, Stapl log.load(); Trigger.timer = new Timer("Hudson cron thread"); - queue = new Queue(); + queue = new Queue(LoadBalancer.DEFAULT); // TODO: make this somehow pluggable try { dependencyGraph = DependencyGraph.EMPTY; diff --git a/core/src/main/java/hudson/model/LoadBalancer.java b/core/src/main/java/hudson/model/LoadBalancer.java new file mode 100644 index 0000000000..3d7f6c2435 --- /dev/null +++ b/core/src/main/java/hudson/model/LoadBalancer.java @@ -0,0 +1,138 @@ +package hudson.model; + +import hudson.model.Queue.Task; +import hudson.model.Queue.JobOffer; +import hudson.model.Node.Mode; + +import java.util.Collection; +import java.util.logging.Logger; + +/** + * Strategy that decides which {@link Task} gets run on which {@link Executor}. + * + * @author Kohsuke Kawaguchi + * @since 1.301 (but this is not yet open for plugins to change) + */ +public abstract class LoadBalancer /*implements ExtensionPoint*/ { + /** + * Chooses the executor to carry out the build for the given project. + * + *

+ * This method is invoked from different threads, but the execution is serialized by the caller. + * The thread that invokes this method always holds a lock to {@link Queue}, so queue contents + * can be safely introspected from this method, if that information is necessary to make + * decisions. + * + * @param available + * The list of {@link JobOffer}s that represent {@linkplain JobOffer#isAvailable() available} {@link Executor}s, from which + * the callee can choose. Never null. + * @param task + * The task whose execution is being considered. Never null. + * + * @return + * Pick one of the items from {@code available}, and return it. How you choose it + * is the crucial part of the implementation. Return null if you don't want + * the task to be executed right now, in which case this method will be called + * some time later with the same task. + */ + protected abstract JobOffer choose(Task task, Collection available); + + /** + * Traditional implementation of this. + */ + public static final LoadBalancer DEFAULT = new LoadBalancer() { + protected JobOffer choose(Task task, Collection available) { + Label l = task.getAssignedLabel(); + if (l != null) { + // if a project has assigned label, it can be only built on it + for (JobOffer offer : available) { + if (l.contains(offer.getNode())) + return offer; + } + return null; + } + + // if we are a large deployment, then we will favor slaves + boolean isLargeHudson = Hudson.getInstance().getNodes().size() > 10; + + // otherwise let's see if the last node where this project was built is available + // it has up-to-date workspace, so that's usually preferable. + // (but we can't use an exclusive node) + Node n = task.getLastBuiltOn(); + if (n != null && n.getMode() == Mode.NORMAL) { + for (JobOffer offer : available) { + if (offer.getNode() == n) { + if (isLargeHudson && offer.getNode() instanceof Slave) + // but if we are a large Hudson, then we really do want to keep the master free from builds + continue; + return offer; + } + } + } + + // duration of a build on a slave tends not to have an impact on + // the master/slave communication, so that means we should favor + // running long jobs on slaves. + // Similarly if we have many slaves, master should be made available + // for HTTP requests and coordination as much as possible + if (isLargeHudson || task.getEstimatedDuration() > 15 * 60 * 1000) { + // consider a long job to be > 15 mins + for (JobOffer offer : available) { + if (offer.getNode() instanceof Slave && offer.isNotExclusive()) + return offer; + } + } + + // lastly, just look for any idle executor + for (JobOffer offer : available) { + if (offer.isNotExclusive()) + return offer; + } + + // nothing available + return null; + } + }; + + /** + * Wraps this {@link LoadBalancer} into a decorator that tests the basic sanity of the implementation. + * Only override this if you find some of the checks excessive, but beware that it's like driving without a seat belt. + */ + protected LoadBalancer sanitize() { + final LoadBalancer base = this; + return new LoadBalancer() { + @Override + protected JobOffer choose(Task task, Collection available) { + if (Hudson.getInstance().isQuietingDown()) { + // if we are quieting down, don't start anything new so that + // all executors will be eventually free. + return null; + } + + JobOffer target = base.choose(task, available); + if(target!=null) { + Label l = task.getAssignedLabel(); + if(l!=null && !l.contains(target.getNode())) { + LOGGER.severe(task.getFullDisplayName()+" needs to be assigned to a node that has a label "+l); + return null; + } + // Queue should guarantee that, but just in case. + assert target.isAvailable(); + } + + return target; + } + + /** + * Double-sanitization is pointless. + */ + @Override + protected LoadBalancer sanitize() { + return this; + } + + private final Logger LOGGER = Logger.getLogger(LoadBalancer.class.getName()); + }; + } + +} diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 77163a6de3..2710c0cfd6 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -118,7 +118,7 @@ public class Queue extends ResourceController implements Saveable { /** * Data structure created for each idle {@link Executor}. - * This is an offer from the queue to an executor. + * This is a job offer from the queue to an executor. * *

* An idle executor (that calls {@link Queue#pop()} creates @@ -168,7 +168,10 @@ public class Queue extends ResourceController implements Saveable { */ private final Map parked = new HashMap(); - public Queue() { + private final LoadBalancer loadBalancer; + + public Queue(LoadBalancer loadBalancer) { + this.loadBalancer = loadBalancer.sanitize(); // if all the executors are busy doing something, then the queue won't be maintained in // timely fashion, so use another thread to make sure it happens. new MaintainTask(this); @@ -569,7 +572,7 @@ public class Queue extends ResourceController implements Saveable { continue; } - JobOffer runner = choose(p.task); + JobOffer runner = loadBalancer.choose(p.task, getAvailableJobOffers()); if (runner == null) // if we couldn't find the executor that fits, // just leave it in the buildables list and @@ -638,66 +641,14 @@ public class Queue extends ResourceController implements Saveable { } /** - * Chooses the executor to carry out the build for the given project. - * - * @return null if no {@link Executor} can run it. + * Creates a sublist of {@link #parked} that only consists of available offers. */ - private JobOffer choose(Task p) { - if (Hudson.getInstance().isQuietingDown()) { - // if we are quieting down, don't start anything new so that - // all executors will be eventually free. - return null; - } - - Label l = p.getAssignedLabel(); - if (l != null) { - // if a project has assigned label, it can be only built on it - for (JobOffer offer : parked.values()) { - if (offer.isAvailable() && l.contains(offer.getNode())) - return offer; - } - return null; - } - - // if we are a large deployment, then we will favor slaves - boolean isLargeHudson = Hudson.getInstance().getNodes().size() > 10; - - // otherwise let's see if the last node where this project was built is available - // it has up-to-date workspace, so that's usually preferable. - // (but we can't use an exclusive node) - Node n = p.getLastBuiltOn(); - if (n != null && n.getMode() == Mode.NORMAL) { - for (JobOffer offer : parked.values()) { - if (offer.isAvailable() && offer.getNode() == n) { - if (isLargeHudson && offer.getNode() instanceof Slave) - // but if we are a large Hudson, then we really do want to keep the master free from builds - continue; - return offer; - } - } - } - - // duration of a build on a slave tends not to have an impact on - // the master/slave communication, so that means we should favor - // running long jobs on slaves. - // Similarly if we have many slaves, master should be made available - // for HTTP requests and coordination as much as possible - if (isLargeHudson || p.getEstimatedDuration() > 15 * 60 * 1000) { - // consider a long job to be > 15 mins - for (JobOffer offer : parked.values()) { - if (offer.isAvailable() && offer.getNode() instanceof Slave && offer.isNotExclusive()) - return offer; - } - } - - // lastly, just look for any idle executor - for (JobOffer offer : parked.values()) { - if (offer.isAvailable() && offer.isNotExclusive()) - return offer; - } - - // nothing available - return null; + private List getAvailableJobOffers() { + List availables = new ArrayList(parked.size()); + for (JobOffer j : parked.values()) + if(j.isAvailable()) + availables.add(j); + return availables; } /** -- GitLab