when sorting nodes for consideration, also take into account last tim…

…e accessed for a more even distribution across nodes over the lifetime of the grid.

Also fix using the RemoteProxy's getResourceUsageInPercent instead of calculating it in the sort.

Fixes #1673

java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java

@@ -563,4 +563,12 @@ private JsonObject extractObject(HttpResponse resp) throws IOException {
   public float getResourceUsageInPercent() {
     return 100 * (float)getTotalUsed() / (float)getMaxNumberOfConcurrentTestSessions();
   }
+
+  public long getLastSessionStart() {
+    long last = -1;
+    for (TestSlot slot : testSlots) {
+      last = Math.max(last, slot.getLastSessionStart());
+    }
+    return last;
+  }
 }

java/server/src/org/openqa/grid/internal/ProxySet.java

@@ -130,10 +130,15 @@ public List<RemoteProxy> getSorted() {
   private Comparator<RemoteProxy> proxyComparator = new Comparator<RemoteProxy>() {
     @Override
     public int compare(RemoteProxy o1, RemoteProxy o2) {
-      double p1used = (o1.getTotalUsed() * 1.0) / o1.getTestSlots().size();
-      double p2used = (o2.getTotalUsed() * 1.0) / o2.getTestSlots().size();
-
-      if (p1used == p2used) return 0;
+      double p1used = o1.getResourceUsageInPercent();
+      double p2used = o2.getResourceUsageInPercent();
+
+      if (p1used == p2used) {
+        long time1lastUsed = o1.getLastSessionStart();
+        long time2lastUsed = o2.getLastSessionStart();
+        if (time1lastUsed == time2lastUsed) return 0;
+        return time1lastUsed < time2lastUsed? -1 : 1;
+      }
       return p1used < p2used? -1 : 1;
     }
   };

java/server/src/org/openqa/grid/internal/RemoteProxy.java

@@ -191,4 +191,9 @@ public interface RemoteProxy extends Comparable<RemoteProxy> {
    * @return the percentage of the available resource used. Can be greater than 100 if the grid is under heavy load.
    */
   float getResourceUsageInPercent();
+
+  /**
+   * @return the time the latest session was started on a TestSlot, -1 if no sessions were started.
+   */
+  long getLastSessionStart();
 }

java/server/src/org/openqa/grid/internal/TestSlot.java

@@ -23,6 +23,7 @@
 import org.openqa.grid.internal.listeners.TestSessionListener;
 import org.openqa.grid.internal.utils.CapabilityMatcher;
 import org.openqa.grid.internal.utils.GridHubConfiguration;
+import org.openqa.selenium.remote.server.SystemClock;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -61,6 +62,7 @@ public class TestSlot {
   private volatile TestSession currentSession;
   volatile boolean beingReleased = false;
   private boolean showWarning = false;
+  private long lastSessionStart = -1;
 
 
   public TestSlot(RemoteProxy proxy, SeleniumProtocol protocol, String path,
@@ -110,6 +112,7 @@ public TestSession getNewSession(Map<String, Object> desiredCapabilities) {
           log.info("Trying to create a new session on test slot " + this.capabilities);
           TestSession session = new TestSession(this, desiredCapabilities, new DefaultTimeSource());
           currentSession = session;
+          lastSessionStart = System.currentTimeMillis();
           return session;
         } else {
           return null;
@@ -247,4 +250,11 @@ public URL getRemoteURL() {
       throw new GridException("Configuration error for the node." + u + " isn't a valid URL");
     }
   }
+
+  /**
+   * @return System.currentTimeMillis() of when the session was started, otherwise -1
+   */
+  public long getLastSessionStart() {
+    return lastSessionStart;
+  }
 }

java/server/test/org/openqa/grid/e2e/misc/GridDistributionTest.java

@@ -35,7 +35,9 @@
 import org.openqa.selenium.server.SeleniumServer;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 public class GridDistributionTest {
 
@@ -114,15 +116,83 @@ public void testLoadIsDistributedEvenly() throws Throwable {
     }
   }
 
-  @AfterClass
-  public static void stop() throws Exception {
+  @Test
+  public void testLeastRecentlyUsedNodesPickedFirst() throws Throwable {
+    ProxySet ps = hub.getRegistry().getAllProxies();
+
+    for (int i=0; i < 4; i++) {
+      drivers.add(GridTestHelper.getRemoteWebDriver(DesiredCapabilities.chrome(), hub));
+    }
+
+    Set<String> chosenNodes = new HashSet<>();
+
+    for (RemoteProxy p : ps) {
+      for (TestSlot ts : p.getTestSlots()) {
+        if (ts.getSession() != null) {
+          chosenNodes.add(p.getRemoteHost().toString());
+          break;
+        }
+      }
+    }
+
+    stopDrivers(drivers);
+
+    for (int i=0; i < 4; i++) {
+      drivers.add(GridTestHelper.getRemoteWebDriver(DesiredCapabilities.chrome(), hub));
+    }
+
+    for (RemoteProxy p : ps) {
+      for (TestSlot ts : p.getTestSlots()) {
+        if (ts.getSession() != null) {
+          Assert.assertFalse("Should not be immediately reused: " + p.getRemoteHost().toString() + " previously used nodes: " + chosenNodes,
+                             chosenNodes.contains(p.getRemoteHost().toString()));
+          break;
+        }
+      }
+    }
+
+    chosenNodes.clear();
+
+    for (RemoteProxy p : ps) {
+      for (TestSlot ts : p.getTestSlots()) {
+        if (ts.getSession() != null) {
+          chosenNodes.add(p.getRemoteHost().toString());
+          break;
+        }
+      }
+    }
+
+    stopDrivers(drivers);
+
+    for (int i=0; i < 4; i++) {
+      drivers.add(GridTestHelper.getRemoteWebDriver(DesiredCapabilities.chrome(), hub));
+    }
+
+    for (RemoteProxy p : ps) {
+      for (TestSlot ts : p.getTestSlots()) {
+        if (ts.getSession() != null) {
+          Assert.assertFalse("Should not be immediately reused: " + p.getRemoteHost().toString() + " previously used nodes: " + chosenNodes,
+                             chosenNodes.contains(p.getRemoteHost().toString()));
+          break;
+        }
+      }
+    }
+  }
+
+  private static void stopDrivers(List<WebDriver> drivers) {
     for (WebDriver driver : drivers) {
       try {
         driver.quit();
       } catch (Exception e) {
         System.out.println(e.toString());
       }
     }
+    drivers.clear();
+  }
+
+  @AfterClass
+  public static void stop() throws Exception {
+    stopDrivers(drivers);
     hub.stop();
   }
 }