Have ExternalServer dynamically find available ports (#3398)

Developers have run into a fair number of issues with servers having
different paths (e.g. if they have two copies of record layer), and thus
not being cleaned up by the store. Instead, it now dynamically picks the
ports (within a range), so if you have some leftover processes sitting
around, new processes will find different ports.

This still leaves the code that kills processes around, so that if you
keep killing tests, you don't end up with hundreds of servers running.

Author: ScottDugas

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlTestExtension.java

@@ -50,10 +50,10 @@
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -97,16 +97,16 @@ public void beforeAll(final ExtensionContext context) throws Exception {
         if (Boolean.parseBoolean(System.getProperty("tests.runQuick", "false"))) {
             testConfigs = List.of(new EmbeddedConfig(clusterFile));
         } else {
-            AtomicInteger serverPort = new AtomicInteger(1111);
             List<File> jars = ExternalServer.getAvailableServers();
             // Fail the test if there are no available servers. This would force the execution in "runQuick" mode in case
             // we don't have access to the artifacts.
             // Potentially, we can relax this a little if all tests are disabled for multi-server execution, but this is
             // not a likely scenario.
             Assertions.assertFalse(jars.isEmpty(), "There are no external servers available to run");
-            servers = jars.stream()
-                    .map(jar -> new ExternalServer(jar, serverPort.getAndIncrement(), serverPort.getAndIncrement(), clusterFile))
-                    .collect(Collectors.toList());
+            servers = new ArrayList<>();
+            for (File jar : jars) {
+                servers.add(new ExternalServer(jar, clusterFile));
+            }
             for (ExternalServer server : servers) {
                 server.start();
             }

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/ExternalServer.java

@@ -25,9 +25,11 @@
 import org.apache.logging.log4j.Logger;
 import org.junit.jupiter.api.Assertions;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.io.File;
 import java.io.IOException;
+import java.net.ServerSocket;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
@@ -44,11 +46,10 @@ public class ExternalServer {
     public static final String EXTERNAL_SERVER_PROPERTY_NAME = "yaml_testing_external_server";
     private static final boolean SAVE_SERVER_OUTPUT = false;
 
-    @Nullable
+    @Nonnull
     private final File serverJar;
-    private final int grpcPort;
-    private final int httpPort;
-    private SemanticVersion version;
+    private int grpcPort;
+    private final SemanticVersion version;
     private Process serverProcess;
     @Nullable
     private final String clusterFile;
@@ -58,21 +59,13 @@ public class ExternalServer {
      *
      * @param serverJar the path to the jar to run
      */
-    public ExternalServer(@Nullable File serverJar, final int grpcPort, final int httpPort) {
-        this(serverJar, grpcPort, httpPort, null);
-    }
+    public ExternalServer(@Nonnull final File serverJar,
+                          @Nullable final String clusterFile) throws IOException {
+        this.clusterFile = clusterFile;
 
-    /**
-     * Create a new instance that will run a specific jar.
-     *
-     * @param serverJar the path to the jar to run
-     */
-    public ExternalServer(@Nullable File serverJar, final int grpcPort, final int httpPort,
-                          @Nullable final String clusterFile) {
         this.serverJar = serverJar;
-        this.grpcPort = grpcPort;
-        this.httpPort = httpPort;
-        this.clusterFile = clusterFile;
+        Assertions.assertTrue(this.serverJar.exists(), "Jar could not be found " + serverJar.getAbsolutePath());
+        this.version = getVersion(this.serverJar);
     }
 
     static {
@@ -113,20 +106,12 @@ public SemanticVersion getVersion() {
     }
 
     public void start() throws Exception {
-        File jar;
-        if (serverJar == null) {
-            final List<File> externalServers = getAvailableServers();
-            Assertions.assertEquals(1, externalServers.size());
-            jar = externalServers.get(0);
-        } else {
-            jar = serverJar;
-        }
-        Assertions.assertTrue(jar.exists(), "Jar could not be found " + jar.getAbsolutePath());
-        this.version = getVersion(jar);
+        grpcPort = getAvailablePort(-1);
+        final int httpPort = getAvailablePort(grpcPort);
         ProcessBuilder processBuilder = new ProcessBuilder("java",
                 // TODO add support for debugging by adding, but need to take care with ports
                 // "-agentlib:jdwp=transport=dt_socket,server=y,address=8000,suspend=n",
-                "-jar", jar.getAbsolutePath(),
+                "-jar", serverJar.getAbsolutePath(),
                 "--grpcPort", Integer.toString(grpcPort), "--httpPort", Integer.toString(httpPort));
         ProcessBuilder.Redirect out = SAVE_SERVER_OUTPUT ?
                                       ProcessBuilder.Redirect.to(File.createTempFile("JdbcServerOut-" + grpcPort, ".log")) :
@@ -144,7 +129,7 @@ public void start() throws Exception {
             Assertions.fail("Failed to start the external server");
         }
 
-        logger.info("Started {} Version: {}", jar, version);
+        logger.info("Started {} Version: {}", serverJar, version);
     }
 
     /**
@@ -201,6 +186,32 @@ private boolean startServer(ProcessBuilder processBuilder) throws IOException, I
         return serverProcess.isAlive();
     }
 
+    /**
+     * Get a port that is currently available for the server.
+     * @param unavailablePort Get a port that you know will be unavailable. This is mostly useful because the server
+     * needs two ports, one for GRPC, and one for HTTP, so the GRPC port can be noted as unavailable when asking for
+     * the http port and both can be provided to the server. If nothing is unavailable, use a negative number.
+     * @return a port that is not currently in use on the system.
+     */
+    private int getAvailablePort(final int unavailablePort) {
+        // running locally on my laptop, testing if a port is available takes 0 milliseconds, so no need to optimize
+        for (int i = 1111; i < 9999; i++) {
+            if (i != unavailablePort && isAvailable(i)) {
+                return i;
+            }
+        }
+        return Assertions.fail("Could not find available port between 1111 and 9999");
+    }
+
+    public static boolean isAvailable(int port) {
+        try (ServerSocket tcpSocket = new ServerSocket(port)) {
+            tcpSocket.setReuseAddress(true);
+            return true;
+        } catch (IOException e) {
+            return false;
+        }
+    }
+
     public void stop() {
         if ((serverProcess != null) && serverProcess.isAlive()) {
             serverProcess.destroy();

yaml-tests/src/test/java/com/apple/foundationdb/relational/yamltests/server/ExternalServerTest.java

@@ -0,0 +1,74 @@
+/*
+ * ExternalServerTest.java
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.apple.foundationdb.relational.yamltests.server;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class ExternalServerTest {
+
+    private File currentServerPath;
+
+    @BeforeEach
+    void setUp() throws IOException {
+        this.currentServerPath = getCurrentServerPath();
+    }
+
+    private static File getCurrentServerPath() throws IOException {
+        final List<File> availableServers = ExternalServer.getAvailableServers();
+        for (File path : availableServers) {
+            if (new ExternalServer(path, null).getVersion().equals(SemanticVersion.current())) {
+                return path;
+            }
+        }
+        return Assertions.fail("No current version server");
+    }
+
+    @Test
+    void startMultiple() throws Exception {
+        final List<ExternalServer> servers = new ArrayList<>();
+        for (int i = 0; i < 3; i++) {
+            servers.add(new ExternalServer(currentServerPath, null));
+        }
+        try {
+            for (final ExternalServer server : servers) {
+                server.start();
+            }
+            // we can't assert about the actual values, because one of the ports may be busy,
+            // so assert that each server has its own port, and none of them have the same port
+            assertEquals(servers.stream().map(ExternalServer::getPort).distinct().collect(Collectors.toList()),
+                    servers.stream().map(ExternalServer::getPort).collect(Collectors.toList()));
+        } finally {
+            for (final ExternalServer server : servers) {
+                server.stop();
+            }
+        }
+    }
+}