Skip to content

Commit d389fc0

Browse files
committed
[grid] Avoiding retries when fetching Node status
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
1 parent 941b8a9 commit d389fc0

File tree

3 files changed

+44
-19
lines changed

3 files changed

+44
-19
lines changed

‎java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,10 @@ public LocalDistributor add(Node node) {
301301
model.add(node.getStatus());
302302
nodes.put(node.getId(), node);
303303
} catch (Exception e) {
304+
LOG.log(
305+
Debug.getDebugLogLevel(),
306+
String.format("Exception while adding Node %s", node.getUri()),
307+
e);
304308
return this;
305309
}
306310

‎java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static org.openqa.selenium.grid.data.Availability.DOWN;
2222
import static org.openqa.selenium.grid.data.Availability.DRAINING;
2323
import static org.openqa.selenium.grid.data.Availability.UP;
24-
import static org.openqa.selenium.net.Urls.fromUri;
2524
import static org.openqa.selenium.remote.http.Contents.asJson;
2625
import static org.openqa.selenium.remote.http.Contents.reader;
2726
import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
@@ -51,6 +50,8 @@
5150
import org.openqa.selenium.json.Json;
5251
import org.openqa.selenium.json.JsonInput;
5352
import org.openqa.selenium.remote.SessionId;
53+
import org.openqa.selenium.remote.http.AddSeleniumUserAgent;
54+
import org.openqa.selenium.remote.http.ClientConfig;
5455
import org.openqa.selenium.remote.http.Filter;
5556
import org.openqa.selenium.remote.http.HttpClient;
5657
import org.openqa.selenium.remote.http.HttpHandler;
@@ -89,7 +90,14 @@ public RemoteNode(
8990
this.externalUri = Require.nonNull("External URI", externalUri);
9091
this.capabilities = ImmutableSet.copyOf(capabilities);
9192

92-
this.client = Require.nonNull("HTTP client factory", clientFactory).createClient(fromUri(externalUri));
93+
HttpClient.Factory httpClientFactory = Require.nonNull("HTTP client factory", clientFactory);
94+
// A client config without RetryRequest(), not needed to monitor the Nodes because the
95+
// health checks implicitly do the retries.
96+
ClientConfig config = ClientConfig
97+
.defaultConfig()
98+
.baseUri(externalUri)
99+
.filter(new AddSeleniumUserAgent());
100+
this.client = httpClientFactory.createClient(config);
93101

94102
this.healthCheck = new RemoteCheck();
95103

‎java/src/org/openqa/selenium/remote/http/ClientConfig.java

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717

1818
package org.openqa.selenium.remote.http;
1919

20+
import org.openqa.selenium.Credentials;
21+
import org.openqa.selenium.internal.Require;
22+
2023
import java.io.UncheckedIOException;
2124
import java.net.MalformedURLException;
2225
import java.net.Proxy;
@@ -25,12 +28,11 @@
2528
import java.net.URL;
2629
import java.time.Duration;
2730

28-
import org.openqa.selenium.Credentials;
29-
import org.openqa.selenium.internal.Require;
30-
3131
public class ClientConfig {
3232

33-
private static final Filter DEFAULT_FILTER = new AddSeleniumUserAgent().andThen(new RetryRequest());
33+
private static final Filter RETRY_FILTER = new RetryRequest();
34+
private static final Filter SELENIUM_USER_AGENT_FILTER = new AddSeleniumUserAgent();
35+
private static final Filter DEFAULT_FILTERS = SELENIUM_USER_AGENT_FILTER.andThen(RETRY_FILTER);
3436
private final URI baseUri;
3537
private final Duration connectionTimeout;
3638
private final Duration readTimeout;
@@ -39,12 +41,12 @@ public class ClientConfig {
3941
private final Credentials credentials;
4042

4143
private ClientConfig(
42-
URI baseUri,
43-
Duration connectionTimeout,
44-
Duration readTimeout,
45-
Filter filters,
46-
Proxy proxy,
47-
Credentials credentials) {
44+
URI baseUri,
45+
Duration connectionTimeout,
46+
Duration readTimeout,
47+
Filter filters,
48+
Proxy proxy,
49+
Credentials credentials) {
4850
this.baseUri = baseUri;
4951
this.connectionTimeout = Require.nonNegative("Connection timeout", connectionTimeout);
5052
this.readTimeout = Require.nonNegative("Read timeout", readTimeout);
@@ -58,7 +60,7 @@ public static ClientConfig defaultConfig() {
5860
null,
5961
Duration.ofSeconds(10),
6062
Duration.ofMinutes(3),
61-
DEFAULT_FILTER,
63+
DEFAULT_FILTERS,
6264
null,
6365
null);
6466
}
@@ -111,14 +113,25 @@ public Duration readTimeout() {
111113
return readTimeout;
112114
}
113115

116+
public ClientConfig filter(Filter filter) {
117+
return new ClientConfig(
118+
baseUri,
119+
connectionTimeout,
120+
readTimeout,
121+
Require.nonNull("Filter", filter),
122+
proxy,
123+
credentials);
124+
}
125+
114126
public ClientConfig withFilter(Filter filter) {
127+
Require.nonNull("Filter", filter);
115128
return new ClientConfig(
116-
baseUri,
117-
connectionTimeout,
118-
readTimeout,
119-
filter == null ? DEFAULT_FILTER : filter.andThen(DEFAULT_FILTER),
120-
proxy,
121-
credentials);
129+
baseUri,
130+
connectionTimeout,
131+
readTimeout,
132+
filter.andThen(DEFAULT_FILTERS),
133+
proxy,
134+
credentials);
122135
}
123136

124137
public Filter filter() {

0 commit comments

Comments
 (0)