Fixing a number of issues, and ensuring that the schedule is processed a bit more correctly

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@14463 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/slaves/SimpleScheduledRetentionStrategy.java

@@ -4,7 +4,7 @@ import java.util.logging.Logger;
 import java.util.logging.Level;
 import java.util.Calendar;
 import java.util.GregorianCalendar;
-import java.util.Date;
+import java.util.concurrent.ExecutionException;
 import java.io.IOException;
 import java.io.ObjectStreamException;
 import java.io.InvalidObjectException;
@@ -12,10 +12,10 @@ import java.io.InvalidObjectException;
 import javax.servlet.ServletException;
 
 import hudson.model.Descriptor;
+import hudson.model.Computer;
 import hudson.scheduler.CronTabList;
 import hudson.util.FormFieldValidator;
 import static hudson.Util.fixNull;
-import hudson.Util;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
@@ -34,22 +34,101 @@ public class SimpleScheduledRetentionStrategy extends RetentionStrategy<SlaveCom
     private final String startTimeSpec;
     private transient CronTabList tabs;
     private transient Calendar lastChecked;
-    private final long upTimeMins;
+    private transient Calendar lastStarted = null;
+    private transient long nextStop = Long.MIN_VALUE;
+    private transient long nextStart = Long.MIN_VALUE;
+    private transient long lastStop = Long.MAX_VALUE;
+    private transient long lastStart = Long.MAX_VALUE;
+    private final int upTimeMins;
+    private final boolean keepUpWhenActive;
 
     @DataBoundConstructor
-    public SimpleScheduledRetentionStrategy(String startTimeSpec, long upTimeMins) throws ANTLRException {
+    public SimpleScheduledRetentionStrategy(String startTimeSpec, int upTimeMins, boolean keepUpWhenActive)
+            throws ANTLRException {
         this.startTimeSpec = startTimeSpec;
+        this.keepUpWhenActive = keepUpWhenActive;
         this.tabs = CronTabList.create(startTimeSpec);
         this.lastChecked = new GregorianCalendar();
-        this.upTimeMins = upTimeMins;
+        this.upTimeMins = Math.max(1, upTimeMins);
         this.lastChecked.add(Calendar.MINUTE, -1);
     }
 
-    protected Object readResolve() throws ObjectStreamException {
+    public int getUpTimeMins() {
+        return upTimeMins;
+    }
+
+    public boolean isKeepUpWhenActive() {
+        return keepUpWhenActive;
+    }
+
+    public String getStartTimeSpec() {
+        return startTimeSpec;
+    }
+
+    private synchronized void updateStartStopWindow() {
+        if (lastStart == Long.MAX_VALUE && lastStop == Long.MAX_VALUE) {
+            // we need to initialize
+
+            // get some useful default values for the lastStart and lastStop... they should be in the past and at least
+            // less than any useful real last start/stop
+            // so default lastStart = now - upTime * 3, and lastStop = now - upTime * 2
+            Calendar time = new GregorianCalendar();
+            time.add(Calendar.MINUTE, -upTimeMins);
+            time.add(Calendar.MINUTE, -upTimeMins);
+            time.add(Calendar.MINUTE, -upTimeMins);
+            lastStart = time.getTimeInMillis();
+            time.add(Calendar.MINUTE, upTimeMins);
+            lastStop = time.getTimeInMillis();
+
+            // we're only interested in the last start if it is less than the upTimeMins ago
+            // any older and last Start is not relevant as the node should be stopped
+            time = new GregorianCalendar();
+            time.add(Calendar.MINUTE, -upTimeMins);
+            time.add(Calendar.MINUTE, -1);
+
+            while (System.currentTimeMillis() + 1000 > time.getTimeInMillis()) {
+                if (tabs.check(time)) {
+                    lastStart = time.getTimeInMillis();
+                    time.add(Calendar.MINUTE, upTimeMins);
+                    lastStop = time.getTimeInMillis();
+                    break;
+                }
+                time.add(Calendar.MINUTE, 1);
+            }
+            nextStart = lastStart;
+            nextStop = lastStop;
+        }
+        if (nextStop < System.currentTimeMillis()) {
+            // next stop is in the past
+            lastStart = nextStart;
+            lastStop = nextStop;
+
+            // we don't want to look too far into the future
+            Calendar time = new GregorianCalendar();
+            time.add(Calendar.MINUTE, Math.min(15, upTimeMins));
+            long stopLooking = time.getTimeInMillis();
+            time.setTimeInMillis(nextStop);
+            while (stopLooking > time.getTimeInMillis()) {
+                if (tabs.check(time)) {
+                    nextStart = time.getTimeInMillis();
+                    time.add(Calendar.MINUTE, upTimeMins);
+                    nextStop = time.getTimeInMillis();
+                    break;
+                }
+                time.add(Calendar.MINUTE, 1);
+            }
+        }
+    }
+
+    protected synchronized Object readResolve() throws ObjectStreamException {
         try {
             tabs = CronTabList.create(startTimeSpec);
             lastChecked = new GregorianCalendar();
             this.lastChecked.add(Calendar.MINUTE, -1);
+            nextStop = Long.MIN_VALUE;
+            nextStart = Long.MIN_VALUE;
+            lastStop = Long.MAX_VALUE;
+            lastStart = Long.MAX_VALUE;
         } catch (ANTLRException e) {
             InvalidObjectException x = new InvalidObjectException(e.getMessage());
             x.initCause(e);
@@ -58,37 +137,64 @@ public class SimpleScheduledRetentionStrategy extends RetentionStrategy<SlaveCom
         return this;
     }
 
-    public long check(SlaveComputer c) {
-        if (c.isOffline()) {
-            Calendar startUp = null;
-            while (new Date().getTime() - lastChecked.getTimeInMillis() > 1000) {
-                LOGGER.log(Level.FINE, "scheduled start checking {0}", lastChecked);
-                if (tabs.check(lastChecked)) {
-                    // need to ensure that we don't fire up after the upTime has been passed
-                    // if we have an aggressive upTime, or a slow check
-                    startUp = (Calendar) lastChecked.clone();
+    public synchronized long check(final SlaveComputer c) {
+        if (lastStarted == null && c.isOnline()) {
+            lastStarted = new GregorianCalendar();
+        } else if (lastStarted != null && c.isOffline()) {
+            lastStarted = null;
+        }
+        updateStartStopWindow();
+        long now = System.currentTimeMillis();
+        boolean shouldBeOnline = (lastStart < now && lastStop > now) || (nextStart < now && nextStop > now);
+        LOGGER.log(Level.INFO, "Checking computer {0} against schedule. online = {1}, shouldBeOnline = {2}",
+                new Object[]{c.getName(), c.isOnline(), shouldBeOnline});
+        if (shouldBeOnline && c.isOffline()) {
+            LOGGER.log(Level.INFO, "Trying to launch computer {0} as schedule says it should be on-line at "
+                    + "this point in time", new Object[]{c.getName()});
+            if (c.isLaunchSupported()) {
+                Computer.threadPoolForRemoting.submit(new Runnable() {
+                    public void run() {
+                        try {
+                            c.connect(true).get();
+                            if (c.isOnline()) {
+                                LOGGER.log(Level.INFO, "Launched computer {0} per schedule", new Object[]{c.getName()});
                             }
-                lastChecked.add(Calendar.MINUTE, 1);
+                            if (keepUpWhenActive && c.isOnline() && !c.isAcceptingTasks()) {
+                                LOGGER.log(Level.INFO,
+                                        "Enabling new jobs for computer {0} as it has started its scheduled uptime",
+                                        new Object[]{c.getName()});
+                                c.setAcceptingTasks(true);
                             }
-            if (startUp != null) {
-                if (new Date().getTime() - startUp.getTimeInMillis() > 1000 * 60L * upTimeMins) {
-                    LOGGER.log(Level.INFO, "Missed scheduled startup");
-                } else {
-                    LOGGER.log(Level.INFO, "Launching computer {0} per schedule", new Object[]{c.getName()});
-                    if (c.isLaunchSupported()) {
-                        c.connect(true);
+                        } catch (InterruptedException e) {
+                        } catch (ExecutionException e) {
                         }
                     }
+                });
+            }
+        } else if (!shouldBeOnline && c.isOnline()) {
+            if (keepUpWhenActive) {
+                if (!c.isIdle() && c.isAcceptingTasks()) {
+                    c.setAcceptingTasks(false);
+                    LOGGER.log(Level.INFO,
+                            "Disabling new jobs for computer {0} as it has finished its scheduled uptime",
+                            new Object[]{c.getName()});
+                    return 1;
+                } else if (c.isIdle() && c.isAcceptingTasks()) {
+                    LOGGER.log(Level.INFO, "Disconnecting computer {0} as it has finished its scheduled uptime",
+                            new Object[]{c.getName()});
+                    c.disconnect();
+                } else if (c.isIdle() && !c.isAcceptingTasks()) {
+                    LOGGER.log(Level.INFO, "Disconnecting computer {0} as it has finished all jobs running when "
+                            + "it completed its scheduled uptime", new Object[]{c.getName()});
+                    c.disconnect();
                 }
-            return upTimeMins;
             } else {
-            if (new Date().getTime() - lastChecked.getTimeInMillis() > 1000 * 60L * upTimeMins) {
                 LOGGER.log(Level.INFO, "Disconnecting computer {0} as it has finished its scheduled uptime",
                         new Object[]{c.getName()});
                 c.disconnect();
             }
-            return 1;
         }
+        return 1;
     }
 
     public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl();
@@ -97,7 +203,7 @@ public class SimpleScheduledRetentionStrategy extends RetentionStrategy<SlaveCom
         return DESCRIPTOR;
     }
 
-    private static class DescriptorImpl extends Descriptor<RetentionStrategy<?>> {
+    public static class DescriptorImpl extends Descriptor<RetentionStrategy<?>> {
 
         /**
          * Constructs a new DescriptorImpl.

core/src/main/resources/hudson/slaves/SimpleScheduledRetentionStrategy/config.jelly

 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout"
          xmlns:t="/lib/hudson" xmlns:f="/lib/form">
     <f:entry title="${%Startup Schedule}" help="/help/project-config/timer-format.html">
-        <f:textarea name="slave.startTimeSpec" checkUrl="'${rootURL}/slaves/SimpleScheduledRetentionStrategy/check?value='+escape(this.value)"
-                    value="${s.startTimeSpec}"/>
+        <f:textarea name="retentionStrategy.startTimeSpec" checkUrl="'${rootURL}/retentionStrategy/SimpleScheduledRetentionStrategy/check?value='+escape(this.value)"
+                    value="${instance.startTimeSpec}"/>
     </f:entry>
     <f:entry title="${%Scheduled Uptime (mins)}">
-        <f:textarea name="slave.upTimeMins" value="${s.upTimeMins}"/>
+        <input class="setting-input number validated" name="retentionStrategy.upTimeMins"
+               type="text" value="${instance.upTimeMins}"
+               checkUrl="'${rootURL}/fieldCheck?errorText='+escape('${%Scheduled Uptime is mandatory.}')+'&amp;value='+escape(this.value)"/>
+    </f:entry>
+    <f:entry title="${%Keep on-line while jobs are running}">
+    <f:checkbox name="retentionStrategy.keepUpWhenActive" checked="${h.defaultToTrue(instance.keepUpWhenActive)}"/>
     </f:entry>
 </j:jelly>
\ No newline at end of file