[grid] Restoring behavior for session creation interval.

A bug was introduced in 4.4 where session creation was
executed every 15s by default. This fix reverts that,
and now session creation is tried every 15 milliseconds.

Additionally, the flag `session-request-timeout-period`
was introduced, to allow users to configure how often
timeouts for new session are checked.

Fixes #10984
Fixes SeleniumHQ/docker-selenium#1690

Author: diemol

java/src/org/openqa/selenium/grid/commands/Hub.java

@@ -149,7 +149,7 @@ protected Handlers createHandlers(Config config) {
     NewSessionQueue queue = new LocalNewSessionQueue(
       tracer,
       distributorOptions.getSlotMatcher(),
-      newSessionRequestOptions.getSessionRequestRetryInterval(),
+      newSessionRequestOptions.getSessionRequestTimeoutPeriod(),
       newSessionRequestOptions.getSessionRequestTimeout(),
       secret);
     handler.addHandler(queue);

java/src/org/openqa/selenium/grid/commands/Standalone.java

@@ -65,7 +65,6 @@
 import java.net.URI;
 import java.net.URL;
 import java.util.Collections;
-import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -149,7 +148,7 @@ protected Handlers createHandlers(Config config) {
     NewSessionQueue queue = new LocalNewSessionQueue(
       tracer,
       distributorOptions.getSlotMatcher(),
-      newSessionRequestOptions.getSessionRequestRetryInterval(),
+      newSessionRequestOptions.getSessionRequestTimeoutPeriod(),
       newSessionRequestOptions.getSessionRequestTimeout(),
       registrationSecret);
     combinedHandler.addHandler(queue);

java/src/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueFlags.java

@@ -17,11 +17,6 @@
 
 package org.openqa.selenium.grid.sessionqueue.config;
 
-import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUE_ROLE;
-import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.DEFAULT_REQUEST_TIMEOUT;
-import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.DEFAULT_RETRY_INTERVAL;
-import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.SESSION_QUEUE_SECTION;
-
 import com.google.auto.service.AutoService;
 
 import com.beust.jcommander.Parameter;
@@ -34,6 +29,12 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static org.openqa.selenium.grid.config.StandardGridRoles.SESSION_QUEUE_ROLE;
+import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.DEFAULT_REQUEST_TIMEOUT;
+import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.DEFAULT_REQUEST_TIMEOUT_PERIOD;
+import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.DEFAULT_RETRY_INTERVAL;
+import static org.openqa.selenium.grid.sessionqueue.config.NewSessionQueueOptions.SESSION_QUEUE_SECTION;
+
 @SuppressWarnings("FieldMayBeFinal")
 @AutoService(HasRoles.class)
 public class NewSessionQueueFlags implements HasRoles {
@@ -63,11 +64,17 @@ public class NewSessionQueueFlags implements HasRoles {
   @ConfigValue(section = SESSION_QUEUE_SECTION, name = "session-request-timeout", example = "5")
   private int sessionRequestTimeout = DEFAULT_REQUEST_TIMEOUT;
 
+  @Parameter(
+    names = {"--session-request-timeout-period"},
+    description = "In seconds, how often the timeout for new session requests is checked.")
+  @ConfigValue(section = SESSION_QUEUE_SECTION, name = "session-request-timeout-period", example = "5")
+  private int sessionRequestTimeoutPeriod = DEFAULT_REQUEST_TIMEOUT_PERIOD;
+
   @Parameter(
     names = {"--session-retry-interval"},
-    description = "Retry interval in seconds. If all slots are busy, new session request " +
-                  "will be retried after the given interval.")
-  @ConfigValue(section = SESSION_QUEUE_SECTION, name = "session-retry-interval", example = "5")
+    description = "Session creation interval in milliseconds. If all slots are busy, new session "
+                  + "request will be retried after the given interval.")
+  @ConfigValue(section = SESSION_QUEUE_SECTION, name = "session-retry-interval", example = "15")
   private int sessionRetryInterval = DEFAULT_RETRY_INTERVAL;
 
   @Override

java/src/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueOptions.java

@@ -35,6 +35,7 @@ public class NewSessionQueueOptions {
 
   static final String SESSION_QUEUE_SECTION = "sessionqueue";
   static final int DEFAULT_REQUEST_TIMEOUT = 300;
+  static final int DEFAULT_REQUEST_TIMEOUT_PERIOD = 10;
   static final int DEFAULT_RETRY_INTERVAL = 15;
 
   private final Config config;
@@ -96,23 +97,33 @@ public Duration getSessionRequestTimeout() {
     return Duration.ofSeconds(timeout);
   }
 
+  public Duration getSessionRequestTimeoutPeriod() {
+    // If the user sets 0 or less, we default to 1s.
+    int timeout = Math.max(
+      config.getInt(SESSION_QUEUE_SECTION, "session-request-timeout-period")
+        .orElse(DEFAULT_REQUEST_TIMEOUT_PERIOD),
+      1);
+
+    return Duration.ofSeconds(timeout);
+  }
+
   public Duration getSessionRequestRetryInterval() {
-    // If the user sets 0 or less, we default to 0s.
+    // If the user sets 0 or less, we default to DEFAULT_RETRY_INTERVAL (in milliseconds).
     int interval = Math.max(
       config.getInt(SESSION_QUEUE_SECTION, "session-retry-interval")
         .orElse(DEFAULT_RETRY_INTERVAL),
-      0);
-    return Duration.ofSeconds(interval);
+      DEFAULT_RETRY_INTERVAL);
+    return Duration.ofMillis(interval);
   }
 
   @ManagedAttribute(name = "RequestTimeoutSeconds")
   public long getRequestTimeoutSeconds() {
     return getSessionRequestTimeout().getSeconds();
   }
 
-  @ManagedAttribute(name = "RetryIntervalSeconds")
-  public long getRetryIntervalSeconds() {
-    return getSessionRequestRetryInterval().getSeconds();
+  @ManagedAttribute(name = "RetryIntervalMilliseconds")
+  public long getRetryIntervalMilliseconds() {
+    return getSessionRequestRetryInterval().toMillis();
   }
 
   public NewSessionQueue getSessionQueue(String implementation) {

java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java

@@ -22,7 +22,6 @@
 import com.google.common.collect.ImmutableSet;
 
 import org.openqa.selenium.Capabilities;
-import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.concurrent.GuardedRunnable;
 import org.openqa.selenium.grid.config.Config;
@@ -114,13 +113,13 @@ public class LocalNewSessionQueue extends NewSessionQueue implements Closeable {
   public LocalNewSessionQueue(
     Tracer tracer,
     SlotMatcher slotMatcher,
-    Duration retryPeriod,
+    Duration requestTimeoutCheck,
     Duration requestTimeout,
     Secret registrationSecret) {
     super(tracer, registrationSecret);
 
     this.slotMatcher = Require.nonNull("Slot matcher", slotMatcher);
-    Require.nonNegative("Retry period", retryPeriod);
+    Require.nonNegative("Retry period", requestTimeoutCheck);
 
     this.requestTimeout = Require.positive("Request timeout", requestTimeout);
 
@@ -130,8 +129,8 @@ public LocalNewSessionQueue(
 
     service.scheduleAtFixedRate(
       GuardedRunnable.guard(this::timeoutSessions),
-      retryPeriod.toMillis(),
-      retryPeriod.toMillis(),
+      requestTimeoutCheck.toMillis(),
+      requestTimeoutCheck.toMillis(),
       MILLISECONDS);
 
     new JMXHelper().register(this);
@@ -148,7 +147,7 @@ public static NewSessionQueue create(Config config) {
     return new LocalNewSessionQueue(
       tracer,
       slotMatcher,
-      newSessionQueueOptions.getSessionRequestRetryInterval(),
+      newSessionQueueOptions.getSessionRequestTimeoutPeriod(),
       newSessionQueueOptions.getSessionRequestTimeout(),
       secretOptions.getRegistrationSecret());
   }

java/test/org/openqa/selenium/grid/router/JmxTest.java

@@ -44,6 +44,13 @@
 import org.openqa.selenium.remote.tracing.DefaultTestTracer;
 import org.openqa.selenium.remote.tracing.Tracer;
 
+import java.lang.management.ManagementFactory;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.logging.Logger;
+
 import javax.management.AttributeNotFoundException;
 import javax.management.InstanceNotFoundException;
 import javax.management.IntrospectionException;
@@ -54,12 +61,6 @@
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 import javax.management.ReflectionException;
-import java.lang.management.ManagementFactory;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.time.Duration;
-import java.time.Instant;
-import java.util.logging.Logger;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.fail;
@@ -163,7 +164,7 @@ void shouldBeAbleToRegisterNode() throws URISyntaxException {
   }
 
   @Test
-  void shouldBeAbleToRegisterSessionQueuerServerConfig() {
+  void shouldBeAbleToRegisterSessionQueueServerConfig() {
     try {
       ObjectName name = new ObjectName(
         "org.seleniumhq.grid:type=Config,name=NewSessionQueueConfig");
@@ -182,9 +183,9 @@ void shouldBeAbleToRegisterSessionQueuerServerConfig() {
       assertThat(Long.parseLong(requestTimeout))
         .isEqualTo(newSessionQueueOptions.getRequestTimeoutSeconds());
 
-      String retryInterval = (String) beanServer.getAttribute(name, "RetryIntervalSeconds");
+      String retryInterval = (String) beanServer.getAttribute(name, "RetryIntervalMilliseconds");
       assertThat(Long.parseLong(retryInterval))
-        .isEqualTo(newSessionQueueOptions.getRetryIntervalSeconds());
+        .isEqualTo(newSessionQueueOptions.getRetryIntervalMilliseconds());
     } catch (InstanceNotFoundException | IntrospectionException | ReflectionException
       | MalformedObjectNameException e) {
       fail("Could not find the registered MBean");