dschneider-pivotal commented on a change in pull request #5935:
URL: https://github.com/apache/geode/pull/5935#discussion_r562142852



##########
File path: 
geode-assembly/src/distributedTest/java/org/apache/geode/rest/internal/web/controllers/RestAPIsAndInterOpsDUnitTest.java
##########
@@ -168,15 +169,18 @@ private CacheServer 
createRegionAndStartCacheServer(String[] regions, Cache cach
   }
 
   private int startManager(final String locators, final String[] regions) 
throws IOException {
+    int[] ports = getRandomAvailableTCPPorts(2);
+    int jmxManagerPort = ports[0];
+    int httpPort = ports[1];
+
     Properties props = new Properties();
     props.setProperty(MCAST_PORT, String.valueOf(0));
     props.setProperty(LOCATORS, locators);
 
     props.setProperty(JMX_MANAGER, "true");
     props.setProperty(JMX_MANAGER_START, "true");
-    props.setProperty(JMX_MANAGER_PORT, "0");
+    props.setProperty(JMX_MANAGER_PORT, String.valueOf(jmxManagerPort));

Review comment:
       In this case you changed from "0" to using getRandomAvailableTCPPorts. I 
would have thought "0" is even better because using getRandomAvailableTCPPorts 
has a small race in which the port it hands back could end up being used by 
another before this thread can bind to it. I thought "0" was better because we 
are telling the operating system to give us an available port at the time we 
bind to it so we get rid of the race window. It can be hard to do in some tests 
when they need to know ahead of time what the actual port is. Do you know using 
"0" causes test issues?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to