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

Reduce HTTP timeout for mailcow API to 5 seconds #83

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Jan 2, 2023

requests timeout is the time a server is allowed not to respond at all, i.e. we only abort the connection if there is no byte received for this duration. For this purpose 30 seconds is too much, it is better to timeout after 5 seconds and restart from scratch. Otherwise within 60 seconds allowed for processing a single new_email requests we can only do 2 attempts, while the limit is 10 attempts.

@link2xt link2xt requested a review from missytake January 2, 2023 03:02
@link2xt
Copy link
Contributor Author

link2xt commented Jan 2, 2023

Now attempts sometimes take ~8 secons instead of usual 1-2 seconds, which looks like a timeout and retry. But we better add more logging like #79 and see if timeouts occur.

Hopefully this fixes the case when worker is killed because it did not respond at all within 60 second timeout.

Copy link
Contributor

@missytake missytake left a comment

Choose a reason for hiding this comment

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

Maybe, as the mailcow API seems to be a bit slow, it makes sense to do something like 10 seconds HTTP timeout and 105 seconds gunicorn worker timeout?

Or reduce the 10 tries to something less?

@link2xt
Copy link
Contributor Author

link2xt commented Jan 2, 2023

Or reduce the 10 tries to something less?

I would reduce it to 2 times. It makes sense to retry once in case of some middlebox (stateful firewall or whatever) hiccup, but if you fail two times it's better to report an error IMO.

@link2xt link2xt force-pushed the link2xt/mailcow-reduce-timeout branch from 9343c3e to 2d5d50b Compare January 2, 2023 12:38
src/mailadm/mailcow.py Outdated Show resolved Hide resolved
Copy link
Contributor

@missytake missytake left a comment

Choose a reason for hiding this comment

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

For instances with many users a larger timeout is necessary here, see other comments :)

src/mailadm/mailcow.py Outdated Show resolved Hide resolved
src/mailadm/mailcow.py Outdated Show resolved Hide resolved
requests timeout is the time a server is allowed not to respond at all,
i.e. we only abort the connection if there is no byte received
for this duration. For this purpose 30 seconds is too much,
it is better to timeout after 5 seconds and restart from scratch.
Otherwise within 60 seconds allowed for processing a single
new_email requests we can only do 2 attempts, while the limit
is 10 attempts.
@link2xt link2xt force-pushed the link2xt/mailcow-reduce-timeout branch from 2d5d50b to c569b1d Compare January 7, 2023 16:56
@link2xt
Copy link
Contributor Author

link2xt commented Jan 7, 2023

I rebased and updated the PR.

@missytake
Copy link
Contributor

CI says there is one lint complaint left, it wants two empty lines after HTTP_TIMEOUT = 5:

  src/mailadm/mailcow.py:5:1: E302 expected 2 blank lines, found 1

@missytake missytake merged commit 9ed555e into master Jan 20, 2023
@missytake missytake deleted the link2xt/mailcow-reduce-timeout branch January 20, 2023 12:18
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