Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race conditions with getRandomPort by storing previous ports #1171

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

tmpolaczyk
Copy link
Contributor

I noticed that sometimes when starting a network with many parachains, one of the nodes seems to fail to start, with the following error seen in the zombienet logs repeatedly:

Error 	 fetching metrics from: http://127.0.0.1:44539/metrics

After inspecting the node logs, it turns out that the prometheus port of one node was being used as the rpc port of another node. Surprisingly, that random port was assigned by zombienet, as you can see in the yaml files:

Collator11.yaml
38:      --prometheus-port 44539 --rpc-port 35281 --ws-port 34043 --collator --

Collator02.yaml
38:      --prometheus-port 35579 --rpc-port 44539 --ws-port 45055 --collator --

So that error can be avoided by storing the ports returned by the getRandomPort function, and ensuring that it doesn't return the same port twice. So that's what is done in this PR, and I have verified it fixes the problem.

@wirednkod
Copy link
Contributor

Thank you @tmpolaczyk . Good catch!

@wirednkod wirednkod requested review from pepoviola and wirednkod July 4, 2023 09:11
@wirednkod wirednkod added the bug Something isn't working label Jul 4, 2023
Copy link
Contributor

@wirednkod wirednkod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the lint issues - lgtm

@tmpolaczyk tmpolaczyk force-pushed the tomasz-get-random-port-race branch from f0706a5 to 48d87e5 Compare July 4, 2023 09:27
@tmpolaczyk
Copy link
Contributor Author

Fixed the eslint errors.

Copy link
Collaborator

@pepoviola pepoviola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!!

@pepoviola
Copy link
Collaborator

Merging, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants