You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by wi...@apache.org on 2018/12/12 22:49:44 UTC
[geode] branch develop updated: GEODE-5674: Stop using random
values for test ports (#2958)
This is an automated email from the ASF dual-hosted git repository.
wirebaron pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new f932299 GEODE-5674: Stop using random values for test ports (#2958)
f932299 is described below
commit f9322996d8f12b136f83162edc4a87699dbbad23
Author: Brian Rowe <br...@pivotal.io>
AuthorDate: Wed Dec 12 14:49:35 2018 -0800
GEODE-5674: Stop using random values for test ports (#2958)
It seems our test framework will fairly frequently have test failing due to
multiple users trying to bind the same port number. This happens when the
different users happen to randomly generate the same port. To keep this from
happening, this change will simply hand out the available ports consecutively
as the test runs. For dunit tests, each child vm will be given a portion of
the overall range for their use.
---
.../AvailablePortHelperIntegrationTest.java | 66 +++++++
.../org/apache/geode/internal/AvailablePort.java | 1 +
.../apache/geode/test/dunit/internal/ChildVM.java | 2 +
.../apache/geode/internal/AvailablePortHelper.java | 203 ++++++++++++++-------
4 files changed, 209 insertions(+), 63 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/AvailablePortHelperIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/AvailablePortHelperIntegrationTest.java
index 7529f38..b6adc70 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/internal/AvailablePortHelperIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/AvailablePortHelperIntegrationTest.java
@@ -17,8 +17,12 @@ package org.apache.geode.internal;
import static org.apache.geode.distributed.internal.DistributionConfig.DEFAULT_MEMBERSHIP_PORT_RANGE;
import static org.apache.geode.internal.AvailablePort.AVAILABLE_PORTS_LOWER_BOUND;
import static org.apache.geode.internal.AvailablePort.AVAILABLE_PORTS_UPPER_BOUND;
+import static org.apache.geode.internal.AvailablePort.MULTICAST;
import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPortRange;
import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPortRangeKeepers;
+import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
+import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableUDPPort;
+import static org.apache.geode.internal.AvailablePortHelper.initializeUniquePortRange;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.IOException;
@@ -224,6 +228,68 @@ public class AvailablePortHelperIntegrationTest {
}
}
+ @Test
+ public void getRandomAvailableUDPPort_succeeds() throws IOException {
+ int udpPort = getRandomAvailableUDPPort();
+ assertThat(udpPort).isNotZero();
+ assertThat(AvailablePort.isPortAvailable(udpPort, MULTICAST)).isTrue();
+ }
+
+ @Test
+ @Parameters({"true", "false"})
+ public void getRandomAvailableTCPPortRange_returnsUniqueRanges(
+ final boolean useMembershipPortRange) {
+ Set<Integer> ports = new HashSet<>();
+
+ for (int i = 0; i < 1000; ++i) {
+ int[] testPorts = getRandomAvailableTCPPortRange(5, useMembershipPortRange);
+ for (int testPort : testPorts) {
+ assertThat(ports).doesNotContain(testPort);
+ ports.add(testPort);
+ }
+ }
+ }
+
+ @Test
+ @Parameters({"true", "false"})
+ public void getRandomAvailableTCPPort_returnsUniqueValues(final boolean useMembershipPortRange) {
+ Set<Integer> ports = new HashSet<>();
+
+ for (int i = 0; i < 1000; ++i) {
+ int testPort = getRandomAvailableTCPPorts(1, useMembershipPortRange)[0];
+ assertThat(ports).doesNotContain(testPort);
+ ports.add(testPort);
+ }
+ }
+
+ @Test
+ @Parameters({"true", "false"})
+ public void initializeUniquePortRange_willReturnSamePortsForSameRange(
+ final boolean useMembershipPortRange) {
+ for (int i = 0; i < 1000; ++i) {
+ initializeUniquePortRange(i);
+ int[] testPorts = getRandomAvailableTCPPorts(3, useMembershipPortRange);
+ initializeUniquePortRange(i);
+ assertThat(getRandomAvailableTCPPorts(3, useMembershipPortRange)).isEqualTo(testPorts);
+ }
+ }
+
+ @Test
+ @Parameters({"true", "false"})
+ public void initializeUniquePortRange_willReturnUniquePortsForUniqueRanges(
+ final boolean useMembershipPortRange) {
+ Set<Integer> ports = new HashSet<>();
+
+ for (int i = 0; i < 1000; ++i) {
+ initializeUniquePortRange(i);
+ int[] testPorts = getRandomAvailableTCPPorts(5, useMembershipPortRange);
+ for (int testPort : testPorts) {
+ assertThat(ports).doesNotContain(testPort);
+ ports.add(testPort);
+ }
+ }
+ }
+
private void assertPortsAreUsable(int[] ports) throws IOException {
for (int port : ports) {
assertPortIsUsable(port);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java b/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java
index cab9b15..a82f779 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/AvailablePort.java
@@ -110,6 +110,7 @@ public class AvailablePort {
InetAddress localHost = SocketCreator.getLocalHost();
socket.setInterface(localHost);
socket.setSoTimeout(Integer.getInteger("AvailablePort.timeout", 2000).intValue());
+ socket.setReuseAddress(true);
byte[] buffer = new byte[4];
buffer[0] = (byte) 'p';
buffer[1] = (byte) 'i';
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ChildVM.java b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ChildVM.java
index c5cb336..ded29cb 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ChildVM.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ChildVM.java
@@ -18,6 +18,7 @@ import java.rmi.Naming;
import org.apache.logging.log4j.Logger;
+import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.ExitCode;
import org.apache.geode.internal.OSProcess;
import org.apache.geode.internal.Version;
@@ -57,6 +58,7 @@ public class ChildVM {
Naming.rebind(name, dunitVM);
JUnit4DistributedTestCase.initializeBlackboard();
holder.signalVMReady();
+ AvailablePortHelper.initializeUniquePortRange(vmNum + 2); // hacky, locator is -2
// This loop is here so this VM will die even if the master is mean killed.
while (!stopMainLoop) {
holder.ping();
diff --git a/geode-junit/src/main/java/org/apache/geode/internal/AvailablePortHelper.java b/geode-junit/src/main/java/org/apache/geode/internal/AvailablePortHelper.java
index 637b52d..10047c2 100644
--- a/geode-junit/src/main/java/org/apache/geode/internal/AvailablePortHelper.java
+++ b/geode-junit/src/main/java/org/apache/geode/internal/AvailablePortHelper.java
@@ -17,16 +17,12 @@ package org.apache.geode.internal;
import static org.apache.geode.distributed.internal.DistributionConfig.DEFAULT_MEMBERSHIP_PORT_RANGE;
import static org.apache.geode.internal.AvailablePort.AVAILABLE_PORTS_LOWER_BOUND;
import static org.apache.geode.internal.AvailablePort.AVAILABLE_PORTS_UPPER_BOUND;
-import static org.apache.geode.internal.AvailablePort.SOCKET;
import static org.apache.geode.internal.AvailablePort.getAddress;
-import static org.apache.geode.internal.AvailablePort.isPortKeepable;
-import java.net.InetAddress;
import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.Iterator;
import java.util.List;
-import java.util.Set;
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.geode.internal.AvailablePort.Keeper;
@@ -38,6 +34,26 @@ import org.apache.geode.internal.AvailablePort.Keeper;
* further calls to getRandomAvailablePort.
*/
public class AvailablePortHelper {
+ private final AtomicInteger currentMembershipPort;
+ private final AtomicInteger currentAvailablePort;
+
+ // Singleton object is only used to track the current ports
+ private static AvailablePortHelper singleton = new AvailablePortHelper();
+
+ AvailablePortHelper() {
+ Random rand;
+ boolean fast = Boolean.getBoolean("AvailablePort.fastRandom");
+ if (fast)
+ rand = new Random();
+ else
+ rand = new java.security.SecureRandom();
+ currentMembershipPort = new AtomicInteger(
+ rand.nextInt(DEFAULT_MEMBERSHIP_PORT_RANGE[1] - DEFAULT_MEMBERSHIP_PORT_RANGE[0])
+ + DEFAULT_MEMBERSHIP_PORT_RANGE[0]);
+ currentAvailablePort =
+ new AtomicInteger(rand.nextInt(AVAILABLE_PORTS_UPPER_BOUND - AVAILABLE_PORTS_LOWER_BOUND)
+ + AVAILABLE_PORTS_LOWER_BOUND);
+ }
/**
* Returns array of unique randomly available tcp ports of specified count.
@@ -73,8 +89,7 @@ public class AvailablePortHelper {
boolean useMembershipPortRange) {
List<Keeper> result = new ArrayList<Keeper>();
while (result.size() < count) {
- result.add(AvailablePort.getRandomAvailablePortKeeper(AvailablePort.SOCKET,
- getAddress(AvailablePort.SOCKET), useMembershipPortRange));
+ result.add(getUniquePortKeeper(useMembershipPortRange, AvailablePort.SOCKET));
}
return result;
}
@@ -85,7 +100,9 @@ public class AvailablePortHelper {
public static int[] getRandomAvailableTCPPortRange(final int count,
final boolean useMembershipPortRange) {
- List<Keeper> list = getRandomAvailableTCPPortRangeKeepers(count, useMembershipPortRange);
+ List<Keeper> list =
+ getUniquePortRangeKeepers(useMembershipPortRange, AvailablePort.SOCKET,
+ count);
int[] ports = new int[list.size()];
int i = 0;
for (Keeper k : list) {
@@ -102,33 +119,8 @@ public class AvailablePortHelper {
public static List<Keeper> getRandomAvailableTCPPortRangeKeepers(final int count,
final boolean useMembershipPortRange) {
- List<Keeper> result = new ArrayList<>();
-
- InetAddress addr = getAddress(AvailablePort.SOCKET);
-
- int lowerBound =
- useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[0] : AVAILABLE_PORTS_LOWER_BOUND;
-
- int upperBound =
- useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[1] : AVAILABLE_PORTS_UPPER_BOUND;
-
- for (int i = lowerBound; i <= upperBound; i++) {
- for (int j = 0; j < count && ((i + j) <= upperBound); j++) {
- int port = i + j;
- Keeper keeper = isPortKeepable(port, SOCKET, addr);
- if (keeper == null) {
- releaseKeepers(result);
- result.clear();
- break;
- }
- result.add(keeper);
- if (result.size() == count) {
- return result;
- }
- }
- }
-
- return result;
+ return getUniquePortRangeKeepers(useMembershipPortRange, AvailablePort.SOCKET,
+ count);
}
private static void releaseKeepers(List<Keeper> keepers) {
@@ -147,16 +139,18 @@ public class AvailablePortHelper {
site = Integer.parseInt(hostName.substring(4));
}
- Set set = new HashSet();
- while (set.size() < count) {
- int port = AvailablePort.getRandomAvailablePortWithMod(AvailablePort.SOCKET, site);
- set.add(new Integer(port));
- }
- int[] ports = new int[set.size()];
+ int[] ports = new int[count];
int i = 0;
- for (Iterator iter = set.iterator(); iter.hasNext();) {
- ports[i] = ((Integer) iter.next()).intValue();
- i++;
+ while (i < count) {
+ int port = getUniquePort(false, AvailablePort.SOCKET);
+ // This logic is from AvailablePort.getRandomAvailablePortWithMod which this method used to
+ // call. It seems like the check should be (port % FOO == site) for some FOO, but given how
+ // widely this is used, it's not at all clear that no one's depending on the current behavior.
+ while (port % site != 0) {
+ port = getUniquePort(false, AvailablePort.SOCKET);
+ }
+ ports[i] = port;
+ ++i;
}
return ports;
}
@@ -166,15 +160,7 @@ public class AvailablePortHelper {
* Returns array of unique randomly available tcp ports of specified count.
*/
public static int getRandomAvailablePortForDUnitSite() {
- int site = 1;
- String hostName = System.getProperty("hostName");
- if (hostName != null && hostName.startsWith("host")) {
- if (hostName.length() > 4) {
- site = Integer.parseInt(hostName.substring(4));
- }
- }
- int port = AvailablePort.getRandomAvailablePortWithMod(AvailablePort.SOCKET, site);
- return port;
+ return getRandomAvailableTCPPortsForDUnitSite(1)[0];
}
@@ -189,16 +175,11 @@ public class AvailablePortHelper {
* Returns array of unique randomly available udp ports of specified count.
*/
public static int[] getRandomAvailableUDPPorts(int count) {
- Set set = new HashSet();
- while (set.size() < count) {
- int port = AvailablePort.getRandomAvailablePort(AvailablePort.MULTICAST);
- set.add(new Integer(port));
- }
- int[] ports = new int[set.size()];
+ int[] ports = new int[count];
int i = 0;
- for (Iterator iter = set.iterator(); iter.hasNext();) {
- ports[i] = ((Integer) iter.next()).intValue();
- i++;
+ while (i < count) {
+ ports[i] = getUniquePort(false, AvailablePort.MULTICAST);
+ ++i;
}
return ports;
}
@@ -210,4 +191,100 @@ public class AvailablePortHelper {
return getRandomAvailableUDPPorts(1)[0];
}
+ public static void initializeUniquePortRange(int rangeNumber) {
+ if (rangeNumber < 0) {
+ throw new RuntimeException("Range number cannot be negative.");
+ }
+ singleton.currentMembershipPort.set(DEFAULT_MEMBERSHIP_PORT_RANGE[0]);
+ singleton.currentAvailablePort.set(AVAILABLE_PORTS_LOWER_BOUND);
+ if (rangeNumber != 0) {
+ // This code will generate starting points such that range 0 starts at the lowest possible
+ // value, range 1 starts halfway through the total available ports, 2 starts 1/4 of the way
+ // through, then further ranges are 3/4, 1/8, 3/8, 5/8, 7/8, 1/16, etc.
+
+ // This spaces the ranges out as much as possible for low numbers of ranges, while also making
+ // it possible to grow the number of ranges without bound (within some reasonable fraction of
+ // the number of available ports)
+ int membershipRange = DEFAULT_MEMBERSHIP_PORT_RANGE[1] - DEFAULT_MEMBERSHIP_PORT_RANGE[0];
+ int availableRange = AVAILABLE_PORTS_UPPER_BOUND - AVAILABLE_PORTS_LOWER_BOUND;
+ int numChunks = Integer.highestOneBit(rangeNumber) << 1;
+ int chunkNumber = 2 * (rangeNumber - Integer.highestOneBit(rangeNumber)) + 1;
+
+ singleton.currentMembershipPort.addAndGet(chunkNumber * membershipRange / numChunks);
+ singleton.currentAvailablePort.addAndGet(chunkNumber * availableRange / numChunks);
+ }
+ }
+
+ /**
+ * Get keeper objects for the next unused, consecutive 'rangeSize' ports on this machine.
+ *
+ * @param useMembershipPortRange - if true, select ports from the
+ * DistributionConfig.DEFAULT_MEMBERSHIP_PORT_RANGE
+ * @param protocol - either AvailablePort.SOCKET (TCP) or AvailablePort.MULTICAST (UDP)
+ * @param rangeSize - number of contiguous ports needed
+ * @return Keeper objects associated with a range of ports satisfying the request
+ */
+ private static List<Keeper> getUniquePortRangeKeepers(boolean useMembershipPortRange,
+ int protocol, int rangeSize) {
+ AtomicInteger targetRange =
+ useMembershipPortRange ? singleton.currentMembershipPort : singleton.currentAvailablePort;
+ int targetBound =
+ useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[1] : AVAILABLE_PORTS_UPPER_BOUND;
+
+ while (true) {
+ int uniquePort = targetRange.getAndAdd(rangeSize);
+ if (uniquePort + rangeSize > targetBound) {
+ targetRange.set(useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[0]
+ : AVAILABLE_PORTS_LOWER_BOUND);
+ continue;
+ }
+ List<Keeper> keepers = new ArrayList<>();
+ int validPortsFound = 0;
+
+ while (validPortsFound < rangeSize) {
+ Keeper testKeeper =
+ AvailablePort.isPortKeepable(uniquePort++, protocol,
+ getAddress(protocol));
+ if (testKeeper == null) {
+ break;
+ }
+
+ keepers.add(testKeeper);
+ ++validPortsFound;
+ }
+
+ if (validPortsFound == rangeSize) {
+ return keepers;
+ }
+
+ releaseKeepers(keepers);
+ }
+ }
+
+ private static Keeper getUniquePortKeeper(boolean useMembershipPortRange, int protocol) {
+ return getUniquePortRangeKeepers(useMembershipPortRange, protocol, 1).get(0);
+ }
+
+ /**
+ * Get the next available port on this machine.
+ */
+ private static int getUniquePort(boolean useMembershipPortRange, int protocol) {
+ AtomicInteger targetRange =
+ useMembershipPortRange ? singleton.currentMembershipPort : singleton.currentAvailablePort;
+ int targetBound =
+ useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[1] : AVAILABLE_PORTS_UPPER_BOUND;
+
+ while (true) {
+ int uniquePort = targetRange.getAndIncrement();
+ if (uniquePort > targetBound) {
+ targetRange.set(useMembershipPortRange ? DEFAULT_MEMBERSHIP_PORT_RANGE[0]
+ : AVAILABLE_PORTS_LOWER_BOUND);
+ continue;
+ }
+
+ if (AvailablePort.isPortAvailable(uniquePort++, protocol, getAddress(protocol))) {
+ return uniquePort;
+ }
+ }
+ }
}