Opened 12 years ago
Closed 12 years ago
#19214 closed New feature (wontfix)
AdminEmailHandler is synchronous.
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Core (Mail) | Version: | 1.4 |
Severity: | Normal | Keywords: | AdminEmailHandler |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi set up my logging settings to email me in certain situations (e.g. aliens visiting my site) by doing logger.warn("Visitor from space")
in a middleware class, and then setting a logger for that middleware class to use AdminEmailHandler. However it is useless, because AdminEmailHandler is synchronous! Any aliens have to wait two or three seconds for the email to be sent before seeing my site!
It would be much much better if the email was asynchronous, or at least there was an additional AsyncAdminEmailHandler which was used by default.
Change History (6)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Component: | Core (Other) → Core (Mail) |
---|---|
Easy pickings: | unset |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → New feature |
There are Django applications dedicated to sending mail asynchronously (https://github.com/pinax/django-mailer/ or http://pypi.python.org/pypi/django-celery-email).
However I think it could be interesting to add an async flag to some Django mail API like mail_admins
or mail_managers
which would do the effective connection to the MTA in a thread (assuming fail_silently=True
).
comment:3 by , 12 years ago
Making it async will only hide the fact that it's slow. Wouldn't it be better to keep the MTA connection open (or have a pool of open connections), as creating the connection is what most likely slows this down? I have a feeling this is a solution on a wrong level - isn't this something that is done in the email backend? And spawning a new thread on each call to emit() looks at least wasteful if not dangerous.
comment:4 by , 12 years ago
I'm very wary of spawning threads in request handlers. It will work in trivial cases but I'm not sure application servers handle this very well under load.
lrekucki's proposal is interesting — and can be implemented as a custom email backend without any change in Django.
comment:5 by , 12 years ago
Yeah... I'm a *HUGE* -1 on this.
Using threads like this is *monunmentally* bad idea. Consider - in the pathological case, your webserver will start up a clean environment for every request. If the thread is allowed to run disconnected, that means you'll have a thread running that you *hope* finishes. Alternatively, you'll need to join the thread before the request finishes, which means you'll end up with the same type of lockup behaviour (although, depending on how it's implemented, it might not be observable to the end-user -- it will just manifest as resource starvation due to an unavailable process on the web server.
In practice. your webserver won't be configured like this; but in practice, you'll also usually have a 'maximum requests served' limit on a process, so you'll never know if you're dealing with the "last" request that a particular process will be handling. You'll end up with the same problem, just harder to peg down.
We have an existing framework for fixing latency problems with sending mail -- it's the mail backend API. The default mail backend is dumb and synchronous. It's a reasonable and default that will work everywhere (once configured) and on low traffic sites will cause no problems. For higher traffic sites, there are plenty of implementations out there that are non-blocking. We don't fix *anything* by making one specific usage threaded -- or even the mail API itself -- threaded and/or asynchronous while still being bound to the request/response cycle.
I'm in favor of wontfixing this; or, at the very most, documenting the synchronous limitations of the default mail handler API.
comment:6 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Simple fix - use the following class instead of the built in AdminEmailHandler in your LOGGING thing.