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

Return 451 code if there is no available threads #64

Open
GoogleCodeExporter opened this issue Mar 30, 2015 · 3 comments
Open

Return 451 code if there is no available threads #64

GoogleCodeExporter opened this issue Mar 30, 2015 · 3 comments

Comments

@GoogleCodeExporter
Copy link
Contributor

What steps will reproduce the problem?
1. Start SMTP server with ExecutorService: 3 threads, 5 queue capacity
2. Try to send email in 30 threads
3. See what happens

What is the expected output? What do you see instead?
expected: SMTP server returns 451 code when pool is empty
actual: SMTP server returns nothing when pool is empty

What version of the product are you using? On what operating system?
version 3.1.7, revision 468

Please provide any additional information below.
Hi there,
IMHO it's not correct behavior that SMTP server returns nothing in this case. I 
think server should say: "I'm busy, try again later". I tried to fix this issue 
for our project. See results of my work in attachment.
Thank you for your attention, Artem.

Original issue reported on code.google.com by [email protected] on 23 Apr 2013 at 6:42

Attachments:

@GoogleCodeExporter
Copy link
Contributor Author

I'm also suffering from the same problem. IMHO server should return 4XX code in 
this case to force sender to resend a mail later, when server will hopefully be 
not that busy.

Original comment by [email protected] on 25 Apr 2013 at 2:22

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Contributor Author

Thanks a lot for this patch, I'll apply it to our project.

Original comment by [email protected] on 26 Apr 2013 at 5:36

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Contributor Author

SubEthaSMTP does send "421 Too many connections, try again later". See the 
source code of Session. 

You do not receive that message, because your test configuration is wrong. You 
have supplied your own ExecutorService with a total capacity of 8 connections, 
including queue, while it seems you left the maxConnections property on its 
default value, which is 1000. And the ExecutorService must actually do the 
tasks, it is not enough to queue them, because the waiting connections will 
eventually timeout. Basically your server will accept 1010 connections and then 
it tries to process them in 3 threads - while it uses blocking IO. This will 
not work out well.

It is not really documented anywhere, and that is indeed an issue, that if you 
replace the built-in ExecutorService, than your own ExecutorService must have a 
capacity of at least about maxConnections + 10.

However, even if a misconfiguration causes this situation, sending an error 
reply is obviously nicer than simply closing the socket. From a semantic 
viewpoint, there is no difference, the client should consider the closing of 
the socket as the equivalent of a 451 reply code. The explicit message makes 
debugging easier though.

The issue here is that we are in the worst place to do anything, in the accept 
loop. It is generally not a good idea to do anything in the accept loop, 
because it is single threaded, so it can be a bottleneck. It is not a big 
difference however. Writing the welcome message takes 0.7 ms, starting a new 
thread takes 0.3 ms on a test Linux virtual machine. Moreover, the specific 
problem at this point is that we cannot start a new task anyway, so a small 
slowdown does not hurt. Assuming that it is really small. I also tested that 
sending a few bytes on a newly opened connection never blocks, so this 
assumption seems to be true.

The only question remains is the code and text of the reply  message. The log 
and and the response together must make it clear that the system is both likely 
overloaded and surely misconfigured.

Original comment by [email protected] on 29 Apr 2013 at 8:21

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant