Tests: check node ports availability when using unicast discovery #9886
Conversation
jaymode
added
>test
| protected static int[] unicastHostPorts(int basePort, int numHosts) { | ||
| int[] unicastHostPorts = new int[numHosts]; | ||
|
|
||
| final int maxPort = basePort + 99; |
There was a problem hiding this comment.
I think we can increase the 99 range to 1000 and also change calcBasePort to ignore the scope component as it is not relevant any more. I want if we should jut remember the last based port and continue from it (round robin-ing) to make in-JVM collisions less likely
|
I like it. Left some comments. Thx for picking it up. |
|
@bleskes thanks for the feedback. I implemented the changes to have a port range of 1000 and to cycle through the range. |
| } | ||
|
|
||
| if (!foundPortInRange) { | ||
| throw new ElasticsearchException("could not find enough open ports in range [" + basePort + "-" + maxPort + "]"); |
There was a problem hiding this comment.
can we also note how ports are required?
There was a problem hiding this comment.
Sure something like could not find enough open ports in range [30000 - 31000]. required [8] open ports; one for each node to bind for tcp transport ?
There was a problem hiding this comment.
I think it's enough to say could not find enough open ports in range [30000 - 31000]. required [8].
|
thx @jaymode - I wonder if we should remember the last port we used and start looking from there? now we start looking from baseport every time so it's likely the first ports we try are still not freed up? |
|
@bleskes we start from |
| @@ -61,24 +63,25 @@ public Settings transportClient() { | |||
|
|
|||
| public static class UnicastZen extends ClusterDiscoveryConfiguration { | |||
|
|
|||
| private static final AtomicInteger portCounter = new AtomicInteger(); | |||
| private static int nextPort = calcBasePort(); | |||
There was a problem hiding this comment.
can we add a comment that this the next port we will try to assign and that it is advanced with every attempt?
|
@jaymode I see I missed it. I left some minor comments. |
|
@bleskes updated with the feedback |
|
LGTM thx! |
Some tests failures are seen when a node attempts to use a port that is already bound by some other process on the test machine. This commit adds a bind to test port availability and iterates over the port range until an available port is found. This reduces the likelihood of a test node failing to start up due to the port already being bound.
jaymode
force-pushed
the
fix/test_unicast_node_ports
branch
from
2c83b10
to
efbda31
Compare
Some tests failures are seen when a node attempts to use a port that is already bound
by some other process on the test machine. This commit adds a bind to test port availability
and iterates over the port range until an available port is found. This reduces the likelihood
of a test node failing to start up due to the port already being bound.