Implement auto port for integration test #327
Conversation
al8n
changed the title
| @@ -294,11 +292,11 @@ fn create_node_config( | |||
| }; | |||
| #[cfg(feature = "waku")] | |||
| { | |||
| config.network.backend.inner.port = Some(get_available_port() as usize); | |||
There was a problem hiding this comment.
how do nodes client know who to contact if the OS decides the port to use? It's a different process, we can't access the address like you do in mixnet node
There was a problem hiding this comment.
This was solved by PR #330. In this PR, we can get the correct address through the MixnodeHandle struct.
There was a problem hiding this comment.
I mean for standard nodes (
nomos-node/tests/src/nodes/nomos.rs
Line 60 in b692043
There was a problem hiding this comment.
Ah, sorry, I forgot the standard node. Yeah, it is hard to get the port for the standard node, I will rethink it. Thanks!
There was a problem hiding this comment.
I also forgot about it. My head was too chaotic. It might be not possible to use auto-assigned ports. If port conflict rarely happen, i think we can go ahead with the current solution. what do you think?
There was a problem hiding this comment.
I actually think #315 made things worse in a way because now you can have conflicts in a single process (previous solution was fine with multiple threads in the same process):
- process 1 ask for some port, receives port 8002, but does not start yet
- process 2 ask for some port, we are unlucky and get port 8002 from the random generator
Since Pick ports more randomly for integration tests #315 is not using any global lock and process 1 has not started, the os reports port 8002 as free, but later they will both try to use it
There was a problem hiding this comment.
Yes. I think that both of previous and current solution have this multiprocess problem, since we don’t have cross-process locking.
I thought that the probability of conflict between processes is lower in the current solution. In the previous solution, all processes start with the port 800x (I don’t remember the exact number). This will cause the high probability of conflict.
For single process, you’re right. We may need to revive the lock or something
There was a problem hiding this comment.
How about reviving the lock from the previous soluton
static NET_PORT: Mutex<u16> = Mutex::new(8000);
but instead of using the constant 8000, what do you think about using a random start port?
Using this lock, the single process would be happy. With a random start port, we can lower the probability of conflicts between processes.
There was a problem hiding this comment.
I'll make a simple PR for this.
There was a problem hiding this comment.
|
I think we can close this PR? since we've reduced the probability of port conflicts by #334 and switched from Jenkins to Github Actions. |
No description provided.