[MINOR] no fixed ports in tests #340
Conversation
Some of our tests rely on fixed network ports being available to run their test. Add code to choose OS assigned ephemiral ports in those cases.
| public static int generateEphemiralPort() throws IOException { | ||
| try (final ServerSocket serverSocket = new ServerSocket(0)) { | ||
| return serverSocket.getLocalPort(); | ||
| } |
There was a problem hiding this comment.
This is unreliable as the port may be used by a different process between this check to find a free port and actually opening the real socket (surprisingly common since the very next port typically assigned is this one). Peer discovery should be able to accept 0 as a port argument and provide a way to get the actual port used which is what we've moved other tests to.
There was a problem hiding this comment.
There are lots of chickens and eggs here. It may involve re-writing how the test works and have two agents talking to each other instead of one talking to itself. While we can have the agent bind to zero, it needs to be peer to itself.
Second, the risk of port re-use is smaller than the certainty of a running node on a default stopping this test.
I'll take a quick gander at a two-agent solution to this test.
There was a problem hiding this comment.
A big concern for me is that this pattern spreads - we used to have a utility method like this and got a ton of intermittency from port conflicts because of it. I'm rather keen to avoid reintroducing the utility method since it has a tendency to spread in its usage. Plus given the number of port conflict problems we had with this approach in the past it doesn't seem to help much.
There was a problem hiding this comment.
Ok, the two agent pattern appears to have worked.
Approach #2, First enable "0" port in PeerDiscoveryAgent so it binds to an open port. Then change the test to use two PeerDiscoveryAgents The change to zero is accomplished by populating the endpoint with the actual bound socket instead of the value passed in. If it is zero the value will change otherwise it will be the value passed in.
PR description
Some of our tests rely on fixed network ports being available to run
their test. Add code to choose OS assigned ephemeral ports in those
cases.