[grid] Avoiding retries when fetching Node status

diemol · diemol · commit d389fc0412e6 · 2022-02-22T00:04:34.000+01:00
By default, http clients have a retry mechanism when connection fails. This is not needed when monitoring the Nodes because the health check is already in charge of that. Fixes SeleniumHQ/docker-selenium#1499
diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -301,6 +301,10 @@ public LocalDistributor add(Node node) {
       model.add(node.getStatus());
       nodes.put(node.getId(), node);
     } catch (Exception e) {
+      LOG.log(
+        Debug.getDebugLogLevel(),
+        String.format("Exception while adding Node %s", node.getUri()),
+        e);
       return this;
     }
 
diff --git a/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java b/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java
@@ -21,7 +21,6 @@
 import static org.openqa.selenium.grid.data.Availability.DOWN;
 import static org.openqa.selenium.grid.data.Availability.DRAINING;
 import static org.openqa.selenium.grid.data.Availability.UP;
-import static org.openqa.selenium.net.Urls.fromUri;
 import static org.openqa.selenium.remote.http.Contents.asJson;
 import static org.openqa.selenium.remote.http.Contents.reader;
 import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
@@ -51,6 +50,8 @@
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.json.JsonInput;
 import org.openqa.selenium.remote.SessionId;
+import org.openqa.selenium.remote.http.AddSeleniumUserAgent;
+import org.openqa.selenium.remote.http.ClientConfig;
 import org.openqa.selenium.remote.http.Filter;
 import org.openqa.selenium.remote.http.HttpClient;
 import org.openqa.selenium.remote.http.HttpHandler;
@@ -89,7 +90,14 @@ public RemoteNode(
     this.externalUri = Require.nonNull("External URI", externalUri);
     this.capabilities = ImmutableSet.copyOf(capabilities);
 
-    this.client = Require.nonNull("HTTP client factory", clientFactory).createClient(fromUri(externalUri));
+    HttpClient.Factory httpClientFactory = Require.nonNull("HTTP client factory", clientFactory);
+    // A client config without RetryRequest(), not needed to monitor the Nodes because the
+    // health checks implicitly do the retries.
+    ClientConfig config = ClientConfig
+      .defaultConfig()
+      .baseUri(externalUri)
+      .filter(new AddSeleniumUserAgent());
+    this.client = httpClientFactory.createClient(config);
 
     this.healthCheck = new RemoteCheck();
 
diff --git a/java/src/org/openqa/selenium/remote/http/ClientConfig.java b/java/src/org/openqa/selenium/remote/http/ClientConfig.java
@@ -17,6 +17,9 @@
 
 package org.openqa.selenium.remote.http;
 
+import org.openqa.selenium.Credentials;
+import org.openqa.selenium.internal.Require;
+
 import java.io.UncheckedIOException;
 import java.net.MalformedURLException;
 import java.net.Proxy;
@@ -25,12 +28,11 @@
 import java.net.URL;
 import java.time.Duration;
 
-import org.openqa.selenium.Credentials;
-import org.openqa.selenium.internal.Require;
-
 public class ClientConfig {
 
-  private static final Filter DEFAULT_FILTER = new AddSeleniumUserAgent().andThen(new RetryRequest());
+  private static final Filter RETRY_FILTER = new RetryRequest();
+  private static final Filter SELENIUM_USER_AGENT_FILTER = new AddSeleniumUserAgent();
+  private static final Filter DEFAULT_FILTERS = SELENIUM_USER_AGENT_FILTER.andThen(RETRY_FILTER);
   private final URI baseUri;
   private final Duration connectionTimeout;
   private final Duration readTimeout;
@@ -39,12 +41,12 @@ public class ClientConfig {
   private final Credentials credentials;
 
   private ClientConfig(
-      URI baseUri,
-      Duration connectionTimeout,
-      Duration readTimeout,
-      Filter filters,
-      Proxy proxy,
-      Credentials credentials) {
+    URI baseUri,
+    Duration connectionTimeout,
+    Duration readTimeout,
+    Filter filters,
+    Proxy proxy,
+    Credentials credentials) {
     this.baseUri = baseUri;
     this.connectionTimeout = Require.nonNegative("Connection timeout", connectionTimeout);
     this.readTimeout = Require.nonNegative("Read timeout", readTimeout);
@@ -58,7 +60,7 @@ public static ClientConfig defaultConfig() {
       null,
       Duration.ofSeconds(10),
       Duration.ofMinutes(3),
-      DEFAULT_FILTER,
+      DEFAULT_FILTERS,
       null,
       null);
   }
@@ -111,14 +113,25 @@ public Duration readTimeout() {
     return readTimeout;
   }
 
+  public ClientConfig filter(Filter filter) {
+    return new ClientConfig(
+      baseUri,
+      connectionTimeout,
+      readTimeout,
+      Require.nonNull("Filter", filter),
+      proxy,
+      credentials);
+  }
+
   public ClientConfig withFilter(Filter filter) {
+    Require.nonNull("Filter", filter);
     return new ClientConfig(
-        baseUri,
-        connectionTimeout,
-        readTimeout,
-        filter == null ? DEFAULT_FILTER : filter.andThen(DEFAULT_FILTER),
-        proxy,
-        credentials);
+      baseUri,
+      connectionTimeout,
+      readTimeout,
+      filter.andThen(DEFAULT_FILTERS),
+      proxy,
+      credentials);
   }
 
   public Filter filter() {
