[grid] Consider max-session value while selecting the slot and identi…

…fying Node capacity (#9838)
SeleniumHQ · Sep 20, 2021 · 46fc208 · 46fc208
1 parent 58c925d
commit 46fc208
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 3 deletions.
diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java
@@ -139,9 +139,10 @@ public boolean hasCapacity() {
     return slots.stream().anyMatch(slot -> slot.getSession() == null);
   }
 
+  // Check if the Node's max session limit is not exceeded and has a free slot that supports the capability.
   public boolean hasCapacity(Capabilities caps) {
-    return slots.stream()
-      .anyMatch(slot -> slot.getSession() == null && slot.isSupporting(caps));
+    return slots.stream().filter(slot -> slot.getSession() != null).count() < maxSessionCount &&
+           slots.stream().anyMatch(slot -> slot.getSession() == null && slot.isSupporting(caps));
   }
 
   public int getMaxSessionCount() {

diff --git a/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java b/java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java
@@ -131,6 +131,37 @@ public void theMostLightlyLoadedNodeIsSelectedFirst() {
     assertThat(lightest.getSlots().stream()).anyMatch(slot -> expected.equals(slot.getId()));
   }
 
+  @Test
+  public void theNodeWhichHasExceededMaxSessionsIsNotSelected() {
+    Capabilities chrome = new ImmutableCapabilities("browserName", "chrome");
+
+    NodeStatus lightLoad =
+      createNode(ImmutableList.of(chrome), 12, 2);
+    NodeStatus mediumLoad =
+      createNode(ImmutableList.of(chrome), 12, 5);
+    NodeStatus maximumLoad =
+      createNode(ImmutableList.of(chrome), 12, 12);
+
+    Set<SlotId> ids = selector.selectSlot(chrome, ImmutableSet.of(maximumLoad, mediumLoad, lightLoad));
+    SlotId expected = ids.iterator().next();
+
+    // The slot should belong to the Node with light load
+    assertThat(lightLoad.getSlots().stream())
+      .anyMatch(slot -> expected.equals(slot.getId()));
+
+    // The node whose current number of sessions is greater than or equal to the max sessions is not included
+    // Hence, the node with the maximum load is skipped
+    ImmutableSet<NodeId> nodeIds = ids.stream()
+      .map(SlotId::getOwningNodeId)
+      .distinct()
+      .collect(toImmutableSet());
+    assertThat(nodeIds).doesNotContain(maximumLoad.getNodeId());
+    assertThat(nodeIds)
+      .containsSequence(
+        lightLoad.getNodeId(),
+        mediumLoad.getNodeId());
+  }
+
   @Test
   public void nodesAreOrderedByNumberOfSupportedBrowsersAndLoad() {
     Capabilities chrome = new ImmutableCapabilities("browserName", "chrome");
@@ -161,6 +192,7 @@ public void nodesAreOrderedByNumberOfSupportedBrowsersAndLoad() {
       .anyMatch(slot -> expected.equals(slot.getId()));
 
     // Nodes are ordered by the diversity of supported browsers, then by load
+    // The node whose current number of sessions is greater than or equal to the max sessions is not included
     ImmutableSet<NodeId> nodeIds = ids.stream()
       .map(SlotId::getOwningNodeId)
       .distinct()
@@ -169,7 +201,6 @@ public void nodesAreOrderedByNumberOfSupportedBrowsersAndLoad() {
       .containsSequence(
         highLoadAndOneBrowser.getNodeId(),
         mediumLoadAndTwoBrowsers.getNodeId(),
-        mediumLoadAndOtherTwoBrowsers.getNodeId(),
         lightLoadAndThreeBrowsers.getNodeId());
   }