Skip to content

Commit 1b23c91

Browse files
diemoldiemol
committed
[grid] Container existence won't be checked.
Checking for container existence does one more request to the docker daemon, and in moments of high usage, this can query the daemon for all containers unnecessarily. Instead of that, we handle the response from the docker endpoint and we log it. Ideally, we are stopping a container when the user said `driver.quit()`, so for the user does not really matter if the container died or not. Logs are enough to investigate issues.
1 parent 7b8bd72 commit 1b23c91

5 files changed

Lines changed: 10 additions & 80 deletions

File tree

‎java/server/src/org/openqa/selenium/docker/Container.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.openqa.selenium.internal.Require;
2121

2222
import java.time.Duration;
23+
import java.util.logging.Level;
2324
import java.util.logging.Logger;
2425

2526
public class Container {
@@ -46,10 +47,11 @@ public void start() {
4647
public void stop(Duration timeout) {
4748
Require.nonNull("Timeout to wait for", timeout);
4849

49-
if (protocol.exists(id)) {
50-
LOG.info("Stopping " + getId());
51-
50+
LOG.info("Stopping " + getId());
51+
try {
5252
protocol.stopContainer(id, timeout);
53+
} catch (RuntimeException e) {
54+
LOG.log(Level.WARNING, "Unable to stop container: " + e.getMessage(), e);
5355
}
5456
}
5557

‎java/server/src/org/openqa/selenium/docker/DockerProtocol.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,5 @@ public interface DockerProtocol {
2727
Container create(ContainerConfig info);
2828
void startContainer(ContainerId id) throws DockerException;
2929
void stopContainer(ContainerId id, Duration timeout) throws DockerException;
30-
boolean exists(ContainerId id);
3130
ContainerInfo inspectContainer(ContainerId id) throws DockerException;
3231
}

‎java/server/src/org/openqa/selenium/docker/v1_40/ContainerExists.java‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

‎java/server/src/org/openqa/selenium/docker/v1_40/V140Docker.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ public class V140Docker implements DockerProtocol {
4040
private final CreateContainer createContainer;
4141
private final StartContainer startContainer;
4242
private final StopContainer stopContainer;
43-
private final ContainerExists containerExists;
4443
private final InspectContainer inspectContainer;
4544

4645
public V140Docker(HttpHandler client) {
@@ -51,7 +50,6 @@ public V140Docker(HttpHandler client) {
5150
createContainer = new CreateContainer(this, client);
5251
startContainer = new StartContainer(client);
5352
stopContainer = new StopContainer(client);
54-
containerExists = new ContainerExists(client);
5553
inspectContainer = new InspectContainer(client);
5654
}
5755

@@ -102,15 +100,6 @@ public void startContainer(ContainerId id) throws DockerException {
102100
startContainer.apply(id);
103101
}
104102

105-
@Override
106-
public boolean exists(ContainerId id) {
107-
Require.nonNull("Container id", id);
108-
109-
LOG.fine(String.format("Checking whether %s is running", id));
110-
111-
return containerExists.apply(id);
112-
}
113-
114103
@Override
115104
public void stopContainer(ContainerId id, Duration timeout) throws DockerException {
116105
Require.nonNull("Container id", id);

‎java/server/src/org/openqa/selenium/grid/docker/DockerSessionFactory.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
133133
attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(),
134134
EventAttribute.setValue(this.getClass().getName()));
135135
LOG.info("Creating container, mapping container port 4444 to " + port);
136-
Map<String, String> browserContainerEnvVars = getBrowserContainerEnvVars(sessionRequest.getCapabilities());
136+
Map<String, String> browserContainerEnvVars =
137+
getBrowserContainerEnvVars(sessionRequest.getCapabilities());
137138
Container container = docker.create(
138139
image(browserImage)
139140
.env(browserContainerEnvVars)
@@ -156,7 +157,9 @@ public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
156157

157158
EXCEPTION.accept(attributeMap, e);
158159
attributeMap.put(AttributeKey.EXCEPTION_MESSAGE.getKey(),
159-
EventAttribute.setValue("Unable to connect to docker server. Stopping container: " + e.getMessage()));
160+
EventAttribute.setValue(
161+
"Unable to connect to docker server. Stopping container: " +
162+
e.getMessage()));
160163
span.addEvent(AttributeKey.EXCEPTION_EVENT.getKey(), attributeMap);
161164

162165
container.stop(Duration.ofMinutes(1));

0 commit comments

Comments
 (0)