-
-
Notifications
You must be signed in to change notification settings - Fork 13
"Flexible timeouts" followup #29
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Timeout should be restarted after successful write operation.
Previously all the commands, such as RCPT and DATA, were using custom user-supplied timeout, but the most expensive operation of sending the message data and waiting for before-queue content filters to finish were using default SMTP timeout. This commit also removes the timeout from message reading operation. It normally happens locally and otherwise I/O operations have their own timeouts, e.g. if the message is read from NFS mount.
dignifiedquire
approved these changes
Aug 4, 2020
iequidoo
added a commit
to chatmail/core
that referenced
this pull request
Dec 21, 2023
Some SMTP servers are running slow before-queue filters, most commonly Postfix with `rspamd` filter which is implemented as a [before-queue Milter](https://www.postfix.org/MILTER_README.html). Some of `rspamd` plugin filters are slow on large mails. We previously had problems with timing out during waiting for SMTP response: #1383. This is largely fixed by chatmail/async-smtp#29 and currently we have 60-second timeout just for reading a response but apparently it is not sufficient -- maybe connection gets killed by NAT while we are waiting for response or `rspamd` takes more than 60 seconds for large messages. As a result a message is resent multiple times and eventually fails with "too many retries" while multiple BCC-self messages are received. We should remove the message from the SMTP queue as soon as we receive it via IMAP as it is clear the message was sent even if we did not manage to get actual SMTP server response.
iequidoo
added a commit
to chatmail/core
that referenced
this pull request
Dec 21, 2023
Some SMTP servers are running slow before-queue filters, most commonly Postfix with `rspamd` filter which is implemented as a [before-queue Milter](https://www.postfix.org/MILTER_README.html). Some of `rspamd` plugin filters are slow on large mails. We previously had problems with timing out during waiting for SMTP response: #1383. This is largely fixed by chatmail/async-smtp#29 and currently we have 60-second timeout just for reading a response but apparently it is not sufficient -- maybe connection gets killed by NAT while we are waiting for response or `rspamd` takes more than 60 seconds for large messages. As a result a message is resent multiple times and eventually fails with "too many retries" while multiple BCC-self messages are received. We should remove the message from the SMTP queue as soon as we receive it via IMAP as it is clear the message was sent even if we did not manage to get actual SMTP server response.
iequidoo
added a commit
to chatmail/core
that referenced
this pull request
Dec 21, 2023
Some SMTP servers are running slow before-queue filters, most commonly Postfix with `rspamd` filter which is implemented as a [before-queue Milter](https://www.postfix.org/MILTER_README.html). Some of `rspamd` plugin filters are slow on large mails. We previously had problems with timing out during waiting for SMTP response: #1383. This is largely fixed by chatmail/async-smtp#29 and currently we have 60-second timeout just for reading a response but apparently it is not sufficient -- maybe connection gets killed by NAT while we are waiting for response or `rspamd` takes more than 60 seconds for large messages. As a result a message is resent multiple times and eventually fails with "too many retries" while multiple BCC-self messages are received. We should remove the message from the SMTP queue as soon as we receive it via IMAP as it is clear the message was sent even if we did not manage to get actual SMTP server response.
iequidoo
added a commit
to chatmail/core
that referenced
this pull request
Dec 21, 2023
Some SMTP servers are running slow before-queue filters, most commonly Postfix with `rspamd` filter which is implemented as a [before-queue Milter](https://www.postfix.org/MILTER_README.html). Some of `rspamd` plugin filters are slow on large mails. We previously had problems with timing out during waiting for SMTP response: #1383. This is largely fixed by chatmail/async-smtp#29 and currently we have 60-second timeout just for reading a response but apparently it is not sufficient -- maybe connection gets killed by NAT while we are waiting for response or `rspamd` takes more than 60 seconds for large messages. As a result a message is resent multiple times and eventually fails with "too many retries" while multiple BCC-self messages are received. We should remove the message from the SMTP queue as soon as we receive it via IMAP as it is clear the message was sent even if we did not manage to get actual SMTP server response.
iequidoo
added a commit
to chatmail/core
that referenced
this pull request
Dec 21, 2023
Some SMTP servers are running slow before-queue filters, most commonly Postfix with `rspamd` filter which is implemented as a [before-queue Milter](https://www.postfix.org/MILTER_README.html). Some of `rspamd` plugin filters are slow on large mails. We previously had problems with timing out during waiting for SMTP response: #1383. This is largely fixed by chatmail/async-smtp#29 and currently we have 60-second timeout just for reading a response but apparently it is not sufficient -- maybe connection gets killed by NAT while we are waiting for response or `rspamd` takes more than 60 seconds for large messages. As a result a message is resent multiple times and eventually fails with "too many retries" while multiple BCC-self messages are received. We should remove the message from the SMTP queue as soon as we receive it via IMAP as it is clear the message was sent even if we did not manage to get actual SMTP server response.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A follow-up for #26
Two commits.
The first one splits timeouts for command writing and response reading, which I think is more robust, because timeout is effectively restarted after successful write operation.
The second one hopefully fixes chatmail/core#1383 for real. Previously custom timeout was applied to all operations except the most expensive one: sending the data and waiting for response. Even worse, after #26 the default timeout (30 seconds in delta chat) was applied to reading the message into the buffer, sending the message, and waiting for response. As a result, sometimes after writing the whole file, 30 seconds were already elapsed, and QUIT was sent immediately after the message instead of waiting for response. Now I have applied the correct timeout and split writing and reading timeouts, like in the first commit.
Commit messages explain the same thing, but without references to delta chat issues and github PRs.