Skip to content

Commit b622c83

Browse files
committed
Reduce port range re-use in tests (#85777)
Today we select the port range to use in tests by taking the Gradle worker ID mod 223. We now use significantly more than 223 Gradle workers in each test run which means port ranges are re-used, resulting in spurious failures. This commit replaces the magic number 223 with a computation defined in terms of the available ports and the number of ports to allocate per worker. It also reduces the number of ports per worker to 30 to try and reduce the frequency of port clashes.
1 parent 3b81473 commit b622c83

File tree

2 files changed

+59
-27
lines changed

2 files changed

+59
-27
lines changed

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,6 @@ public abstract class ESTestCase extends LuceneTestCase {
197197
private static final AtomicInteger portGenerator = new AtomicInteger();
198198

199199
private static final Collection<String> loggedLeaks = new ArrayList<>();
200-
public static final int MIN_PRIVATE_PORT = 13301;
201200

202201
private HeaderWarningAppender headerWarningAppender;
203202

@@ -1708,38 +1707,71 @@ public static boolean inFipsJvm() {
17081707
return Boolean.parseBoolean(System.getProperty(FIPS_SYSPROP));
17091708
}
17101709

1710+
/*
1711+
* [NOTE: Port ranges for tests]
1712+
*
1713+
* Some tests involve interactions over the localhost interface of the machine running the tests. The tests run concurrently in multiple
1714+
* JVMs, but all have access to the same network, so there's a risk that different tests will interact with each other in unexpected
1715+
* ways and trigger spurious failures. Gradle numbers its workers sequentially starting at 1 and each worker can determine its own
1716+
* identity from the {@link #TEST_WORKER_SYS_PROPERTY} system property. We use this to try and assign disjoint port ranges to each test
1717+
* worker, avoiding any unexpected interactions, although if we spawn enough test workers then we will wrap around to the beginning
1718+
* again.
1719+
*/
1720+
17111721
/**
1712-
* Returns a unique port range for this JVM starting from the computed base port
1722+
* Defines the size of the port range assigned to each worker, which must be large enough to supply enough ports to run the tests, but
1723+
* not so large that we run out of ports. See also [NOTE: Port ranges for tests].
17131724
*/
1714-
public static String getPortRange() {
1715-
return getBasePort() + "-" + (getBasePort() + 99); // upper bound is inclusive
1725+
private static final int PORTS_PER_WORKER = 30;
1726+
1727+
/**
1728+
* Defines the minimum port that test workers should use. See also [NOTE: Port ranges for tests].
1729+
*/
1730+
protected static final int MIN_PRIVATE_PORT = 13301;
1731+
1732+
/**
1733+
* Defines the maximum port that test workers should use. See also [NOTE: Port ranges for tests].
1734+
*/
1735+
private static final int MAX_PRIVATE_PORT = 36600;
1736+
1737+
/**
1738+
* Wrap around after reaching this worker ID.
1739+
*/
1740+
private static final int MAX_EFFECTIVE_WORKER_ID = (MAX_PRIVATE_PORT - MIN_PRIVATE_PORT - PORTS_PER_WORKER + 1) / PORTS_PER_WORKER - 1;
1741+
1742+
static {
1743+
assert getWorkerBasePort(MAX_EFFECTIVE_WORKER_ID) + PORTS_PER_WORKER - 1 <= MAX_PRIVATE_PORT;
17161744
}
17171745

1718-
protected static int getBasePort() {
1719-
// some tests use MockTransportService to do network based testing. Yet, we run tests in multiple JVMs that means
1720-
// concurrent tests could claim port that another JVM just released and if that test tries to simulate a disconnect it might
1721-
// be smart enough to re-connect depending on what is tested. To reduce the risk, since this is very hard to debug we use
1722-
// a different default port range per JVM unless the incoming settings override it
1723-
// use a non-default base port otherwise some cluster in this JVM might reuse a port
1746+
/**
1747+
* Returns a port range for this JVM according to its Gradle worker ID. See also [NOTE: Port ranges for tests].
1748+
*/
1749+
public static String getPortRange() {
1750+
final int firstPort = getWorkerBasePort();
1751+
final int lastPort = firstPort + PORTS_PER_WORKER - 1; // upper bound is inclusive
1752+
assert MIN_PRIVATE_PORT <= firstPort && lastPort <= MAX_PRIVATE_PORT;
1753+
return firstPort + "-" + lastPort;
1754+
}
17241755

1725-
// We rely on Gradle implementation details here, the worker IDs are long values incremented by one for the
1726-
// lifespan of the daemon this means that they can get larger than the allowed port range.
1727-
// Ephemeral ports on Linux start at 32768 so we modulo to make sure that we don't exceed that.
1728-
// This is safe as long as we have fewer than 224 Gradle workers running in parallel
1729-
// See also: https://github.com/elastic/elasticsearch/issues/44134
1756+
/**
1757+
* Returns the start of the port range for this JVM according to its Gradle worker ID. See also [NOTE: Port ranges for tests].
1758+
*/
1759+
protected static int getWorkerBasePort() {
17301760
final String workerIdStr = System.getProperty(ESTestCase.TEST_WORKER_SYS_PROPERTY);
1731-
final int startAt;
17321761
if (workerIdStr == null) {
1733-
startAt = 0; // IDE
1734-
} else {
1735-
// we adjust the gradle worker id with mod so as to not go over the ephemoral port ranges, but gradle continually
1736-
// increases this value, so the mod can eventually become zero, thus we shift on both sides by 1
1737-
final long workerId = Long.valueOf(workerIdStr);
1738-
assert workerId >= 1 : "Non positive gradle worker id: " + workerIdStr;
1739-
startAt = (int) Math.floorMod(workerId - 1, 223L) + 1;
1762+
// running in IDE
1763+
return MIN_PRIVATE_PORT;
17401764
}
1741-
assert startAt >= 0 : "Unexpected test worker Id, resulting port range would be negative";
1742-
return MIN_PRIVATE_PORT + (startAt * 100);
1765+
1766+
final int workerId = Integer.parseInt(workerIdStr);
1767+
assert workerId >= 1 : "Non positive gradle worker id: " + workerIdStr;
1768+
return getWorkerBasePort(workerId % (MAX_EFFECTIVE_WORKER_ID + 1));
1769+
}
1770+
1771+
private static int getWorkerBasePort(int effectiveWorkerId) {
1772+
assert 0 <= effectiveWorkerId && effectiveWorkerId <= MAX_EFFECTIVE_WORKER_ID;
1773+
// the range [MIN_PRIVATE_PORT, MIN_PRIVATE_PORT+PORTS_PER_WORKER) is only for running outside of Gradle
1774+
return MIN_PRIVATE_PORT + PORTS_PER_WORKER + effectiveWorkerId * PORTS_PER_WORKER;
17431775
}
17441776

17451777
protected static InetAddress randomIp(boolean v4) {

test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,12 @@ public void testWorkerSystemProperty() {
180180
public void testBasePortGradle() {
181181
assumeTrue("requires running tests with Gradle", System.getProperty("tests.gradle") != null);
182182
// Gradle worker IDs are 1 based
183-
assertNotEquals(10300, ESTestCase.getBasePort());
183+
assertNotEquals(ESTestCase.MIN_PRIVATE_PORT, ESTestCase.getWorkerBasePort());
184184
}
185185

186186
public void testBasePortIDE() {
187187
assumeTrue("requires running tests without Gradle", System.getProperty("tests.gradle") == null);
188-
assertEquals(10300, ESTestCase.getBasePort());
188+
assertEquals(ESTestCase.MIN_PRIVATE_PORT, ESTestCase.getWorkerBasePort());
189189
}
190190

191191
public void testRandomDateFormatterPattern() {

0 commit comments

Comments
 (0)