honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix port range allocation with large worker IDs (#44213) 

* Fix port range allocation with large worker IDs

Relates to #43983

The IDs gradle uses are incremented for the lifetime of the daemon which
can result in port ranges that are outside the valid range.
This change implements a modulo based formula to wrap the port ranges
when the IDs get too large.

Adresses #44134 but #44157 is also required to be able to close it.
This commit is contained in:
Alpar Torok 2019-07-12 10:53:33 +03:00
parent d2407d0ffc
commit 8d35583c43
5 changed files with 65 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java
View File

@ -187,14 +187,14 @@ public abstract class ESTestCase extends LuceneTestCase {
}
// Allows distinguishing between parallel test processes
public static final int TEST_WORKER_VM;
public static final String TEST_WORKER_VM_ID;
protected static final String TEST_WORKER_SYS_PROPERTY = "org.gradle.test.worker";
public static final String TEST_WORKER_SYS_PROPERTY = "org.gradle.test.worker";
public static final String DEFAULT_TEST_WORKER_ID = "--not-gradle--";
static {
// org.gradle.test.worker starts counting at 1, but we want to start counting at 0 here
// in case system property is not defined (e.g. when running test from IDE), just use 0
TEST_WORKER_VM = RandomizedTest.systemPropertyAsInt(TEST_WORKER_SYS_PROPERTY, 1) - 1;
TEST_WORKER_VM_ID = System.getProperty(TEST_WORKER_SYS_PROPERTY, DEFAULT_TEST_WORKER_ID);
setTestSysProps();
LogConfigurator.loadLog4jPlugins();

2
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
View File

@ -525,7 +525,7 @@ public final class InternalTestCluster extends TestCluster {
public static String clusterName(String prefix, long clusterSeed) {
StringBuilder builder = new StringBuilder(prefix);
builder.append("-TEST_WORKER_VM=[").append(ESTestCase.TEST_WORKER_VM).append(']');
builder.append("-TEST_WORKER_VM=[").append(ESTestCase.TEST_WORKER_VM_ID).append(']');
builder.append("-CLUSTER_SEED=[").append(clusterSeed).append(']');
// if multiple maven task run on a single host we better have an identifier that doesn't rely on input params
builder.append("-HASH=[").append(SeedUtils.formatSeed(System.nanoTime())).append(']');

39
test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java
View File

@ -106,27 +106,28 @@ public final class MockTransportService extends TransportService {
}
/**
* Some tests use MockTransportService to do network based testing. Yet, we run tests in multiple JVMs that means
* concurrent tests could claim port that another JVM just released and if that test tries to simulate a disconnect it might
* be smart enough to re-connect depending on what is tested. To reduce the risk, since this is very hard to debug we use
* a different default port range per JVM unless the incoming settings override it
* use a non-default base port otherwise some cluster in this JVM might reuse a port
*/
private static int getBasePort() {
final int basePort = 10300 + (ESTestCase.TEST_WORKER_VM * 100);
if (basePort < 10300 || basePort >= 65000) {
// to ensure we don't get illegal ports above 65536 in the getPortRange method
throw new AssertionError("Expected basePort to be between 10300 and 65000 but was " + basePort);
}
return basePort;
}
/**
* Returns a unique port range for this JVM starting from the computed base port (see {@link #getBasePort()})
* Returns a unique port range for this JVM starting from the computed base port
*/
public static String getPortRange() {
int basePort = getBasePort();
return basePort + "-" + (basePort + 99); // upper bound is inclusive
return getBasePort() + "-" + (getBasePort() + 99); // upper bound is inclusive
}
protected static int getBasePort() {
// some tests use MockTransportService to do network based testing. Yet, we run tests in multiple JVMs that means
// concurrent tests could claim port that another JVM just released and if that test tries to simulate a disconnect it might
// be smart enough to re-connect depending on what is tested. To reduce the risk, since this is very hard to debug we use
// a different default port range per JVM unless the incoming settings override it
// use a non-default base port otherwise some cluster in this JVM might reuse a port
// We rely on Gradle implementation details here, the worker IDs are long values incremented by one for the
// lifespan of the daemon this means that they can get larger than the allowed port range.
// Ephemeral ports on Linux start at 32768 so we modulo to make sure that we don't exceed that.
// This is safe as long as we have fewer than 224 Gradle workers running in parallel
// See also: https://github.com/elastic/elasticsearch/issues/44134
final String workerId = System.getProperty(ESTestCase.TEST_WORKER_SYS_PROPERTY);
final int startAt = workerId == null ? 0 : (int) Math.floorMod(Long.valueOf(workerId), 223);
assert startAt >= 0 : "Unexpected test worker Id, resulting port range would be negative";
return 10300 + (startAt * 100);
}
public static MockNioTransport newMockTransport(Settings settings, Version version, ThreadPool threadPool) {

7
test/framework/src/test/java/org/elasticsearch/test/test/ESTestCaseTests.java
View File

@ -19,7 +19,6 @@
package org.elasticsearch.test.test;
import com.carrotsearch.randomizedtesting.RandomizedTest;
import junit.framework.AssertionFailedError;
import org.elasticsearch.common.bytes.BytesReference;
@ -43,6 +42,7 @@ import java.util.function.Supplier;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
public class ESTestCaseTests extends ESTestCase {
@ -185,8 +185,7 @@ public class ESTestCaseTests extends ESTestCase {
public void testWorkerSystemProperty() {
assumeTrue("requires running tests with Gradle", System.getProperty("tests.gradle") != null);
// org.gradle.test.worker starts counting at 1
assertThat(RandomizedTest.systemPropertyAsInt(TEST_WORKER_SYS_PROPERTY, -1), greaterThan(0));
assertEquals(RandomizedTest.systemPropertyAsInt(TEST_WORKER_SYS_PROPERTY, -1) - 1, TEST_WORKER_VM);
assertThat(ESTestCase.TEST_WORKER_VM_ID, not(equals(ESTestCase.DEFAULT_TEST_WORKER_ID)));
}
}

36
test/framework/src/test/java/org/elasticsearch/test/transport/MockTransportServiceTests.java Normal file
View File

@ -0,0 +1,36 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you 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 org.elasticsearch.test.transport;
import org.elasticsearch.test.ESTestCase;
public class MockTransportServiceTests extends ESTestCase {
public void testBasePortGradle() {
assumeTrue("requires running tests with Gradle", System.getProperty("tests.gradle") != null);
// Gradle worker IDs are 1 based
assertNotEquals(10300, MockTransportService.getBasePort());
}
public void testBasePortIDE() {
assumeTrue("requires running tests without Gradle", System.getProperty("tests.gradle") == null);
assertEquals(10300, MockTransportService.getBasePort());
}
}