voodoodyne / subethasmtp

SubEtha SMTP is a Java library for receiving SMTP mail
Other
343 stars 138 forks source link

Return 451 code if there is no available threads #64

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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 art....@gmail.com on 23 Apr 2013 at 6:42

Attachments:

GoogleCodeExporter commented 9 years ago
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 eugen...@gmail.com on 25 Apr 2013 at 2:22

GoogleCodeExporter commented 9 years ago
Thanks a lot for this patch, I'll apply it to our project.

Original comment by alexsim...@gmail.com on 26 Apr 2013 at 5:36

GoogleCodeExporter commented 9 years ago
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 hontvari@flyordie.com on 29 Apr 2013 at 8:21