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

Log a warning for failed attempts to connect to master #2136

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

gdm85
Copy link
Contributor

@gdm85 gdm85 commented Jul 21, 2022

The connection timeout and number of attempts are hardcoded (60 attempts with an interval of 5 seconds each), so a complete failure of the worker to connect to master will take very long.

These log lines will allow to troubleshoot issues with the connection to master.

@cyberw
Copy link
Collaborator

cyberw commented Jul 21, 2022

Maybe make it debug the first 2 times and then warning level? Dont wanna spam the user in what is a normal scenario (master is often not available at the very start), and error should be reserved for catastrophic fails.

@gdm85
Copy link
Contributor Author

gdm85 commented Jul 21, 2022

Sure, I will change it as I am not familiar with how log levels are used in Locust and this change should indeed not break consistency.

Dont wanna spam the user in what is a normal scenario (master is often not available at the very start)

"2" is not a magic number that will cover all situations, I'd rather have everything with same level but will do as you suggested for now. I would probably make retries and delay between retries configurable (in a separate PR), but at least with this one the on-going failures will stop being completely concealed.

@cyberw
Copy link
Collaborator

cyberw commented Jul 24, 2022

Lgtm, except for the formatting/black issue.

The connection timeout and number of attempts are hardcoded, so a failure will take very long
These log lines will allow to troubleshoot issues with the connection to master
@gdm85
Copy link
Contributor Author

gdm85 commented Jul 25, 2022

Oh, my bad, I had missed that; fixed now.

@cyberw cyberw merged commit cc87816 into locustio:master Jul 25, 2022
@cyberw cyberw changed the title Log an error for every failed attempt to connect to master Log a warning for failed attempts to connect to master Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants