test: dynamic port in cluster worker dgram #12487
Conversation
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. Refs: #12376
mscdex
added
cluster
| @@ -111,5 +111,5 @@ function worker() { | |||
| } | |||
| }, PACKETS_PER_WORKER)); | |||
|
|
|||
| socket.bind(common.PORT); | |||
| socket.bind(0); | |||
There was a problem hiding this comment.
I don't think this is right. I think each of the four workers will get a different port with this change. In the current test, they are all using the same port and I think that behavior should be preserved. Otherwise, all the messages go to a single worker.
There was a problem hiding this comment.
@Trott I think they'll be using the same port because exclusive defaults to false and the fd will be shared among workers
There was a problem hiding this comment.
@santigimeno Debugging indicates you are correct. Clearing my review. Thanks.
There was a problem hiding this comment.
@santigimeno sorry, but mind explaining why this is correct?
There was a problem hiding this comment.
@benjamingr, from the dgram documentation:
The options object may contain an additional exclusive property that is use when using dgram.Socket objects with the cluster module. When exclusive is set to false (the default), cluster workers will use the same underlying socket handle allowing connection handling duties to be shared. When exclusive is true, however, the handle is not shared and attempted port sharing results in an error.
I think this is how it works:
When a worker binds a UDP socket in non-exclusive mode, it requests the master process for a valid socket handle by sending a queryServer cluster message with the socket info (key). The master will create the socket handle only after receiving the first message with that specific key and then send the handle back to the worker. The following messages from the workers requesting a socket handle with the same key, will cause the master to send the same handle back to every worker.
|
Any reason this shouldn't land? |
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: nodejs#12487 Ref: nodejs#12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
|
Landed in cf68280. Thanks for the contribution! |
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: #12487 Ref: #12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: #12487 Ref: #12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: #12487 Ref: #12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: #12487 Ref: #12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: #12487 Ref: #12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and test-cluster-dgram-2, in order to eliminate the possibility of port collision. PR-URL: nodejs/node#12487 Ref: nodejs/node#12376 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
Remove common.PORT from test-cluster-dgram-1 and
test-cluster-dgram-2, in order to eliminate the
possibility of port collision.
Refs: #12376
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test