1

The MVC project comes with partially built-in email services. I have implemented the EmailService class that is working fine. However, I need to send the emails in bulk to multiple users thus I would like to run the process in the background and this is where I'm stuck. I have tried the following codes:

//To simplify the example, I will just use the loop
for (int i = 1; i <= 10; i++)
{
    //Version 1:
    await UserManager.SendEmailAsync(userId + i, "Test subject line", "Sample email body");

    //Version 2:
    UserManager.SendEmailAsync(userId + i, "Test subject line", "Sample email body");

    //Version 3:
    ThreadPool.QueueUserWorkItem(o => UserManager.SendEmailAsync(userId + i, "Test subject line", "Sample email body"));

    //Version 4:
    Task.Run(async () => { await UserManager.SendEmailAsync(userId + i, "Test subject line", "Sample email body"); });

    //Version 5:
    await Task.Run(() => { UserManager.SendEmailAsync(userId + i, "Test subject line", "Sample email body"); });
}

Version 1 is the only working code. But the await will block the code which causes long delay before moving to the next page.

Version 2, 3 and 4 don't work at all.

Version 5 has a very odd behavior. I always get one less email. Using the i to track which email I'm missing, it's always the last one that is missing. (in above example, I will always receive email from userId 1 to 9 only.) And if I try to send only one email, then I can never receive one.

What went wrong?

Edited Thanks everyone for your response, but I probably should mention that:

  1. At the time I want to "fire and forget', I am almost certain that I have everything I need, and any process after that does not depend on this process.

  2. I do NOT care the success/failure of this process. (Will probably log the error, but would not care much if it fails)

C.J.
  • 3,409
  • 8
  • 34
  • 51
  • I'm not exactly sure what you mean by the first Version blocks your code, but how about simply removing the `await`? As I've understood you want to send the page content to the user as fast as possible, without having him to wait till all emails are send right? – Florian Moser Mar 28 '16 at 15:55
  • You're closing over the loop variable. There are several posts explaining this. Here is [one](http://stackoverflow.com/questions/451779/how-to-tell-a-lambda-function-to-capture-a-copy-instead-of-a-reference-in-c). Foreach loop is changed in c# 5.0, but for stays the same. You need to take a copy of the variable `i` to fix it. – Sriram Sakthivel Mar 28 '16 at 15:57
  • Possible duplicate of [Is there a reason for C#'s reuse of the variable in a foreach?](http://stackoverflow.com/questions/8898925/is-there-a-reason-for-cs-reuse-of-the-variable-in-a-foreach) – Sriram Sakthivel Mar 28 '16 at 15:58
  • 1
    @SriramSakthivel how is this question even the same as the one you're mentioning? – C.J. Mar 28 '16 at 16:10
  • @C.J. I explained that already in [above comment](http://stackoverflow.com/questions/36265277/sendemailasync-in-background-process?noredirect=1#comment60159568_36265277) – Sriram Sakthivel Mar 28 '16 at 16:19
  • @FlorianMoser in my example above, the code has to wait till it finishes sending all 10 emails before redirecting the users to the next step of the process. Without the `await` keyword, it will just somehow "skip" that line and does nothing (no email is being sent) – C.J. Mar 28 '16 at 16:22
  • @C.J. Yes my bad, of course it does. It just skips the line, putting it in line to execute later, but as soon as the response is sent, it will have "forgotten" about it. This is kind of http is made: You create a request, and get a response. Between the responses nothing's happening. I think I've found a solution for you (or Stephen Cleary did): http://blog.stephencleary.com/2012/12/returning-early-from-aspnet-requests.html – Florian Moser Mar 28 '16 at 16:23
  • @SriramSakthivel Good catch, but it is still unrelated to this question. My question is "how to run SendEmailAsync in the background", I added that `i` on the fly when posting the question to remove some sensitive information in the actual codes. – C.J. Mar 28 '16 at 16:25
  • 2
    Found the problem: `SendEmailAsync in background process... The MVC project`. ASP.NET does not support reliable "background processes" (that is, code that executes outside of a request context). – Stephen Cleary Mar 28 '16 at 18:01
  • 1
    @C.J. - I know its not what you want to hear (sorry) but I do not think this will change the answers. ASP.NET (including MVC as it sits on top of asp.net) was just not designed with the goal of running long running requests. You also mention logging if it fails, with a long running request like this there is no guarantee that logging will occur either. An app pool recycle is the equivalent of Process.Kill(), there is no cleanup code. – Igor Mar 28 '16 at 19:25

2 Answers2

3

...run the process in the background and this is where I'm stuck

You should not execute long running requests that are critical using a web request. The number one reason is you have to plan for failure. What happens if the application pool is recycled? In that case the work could be lost and you might note have the state necessary to pick up execution where it left off (also how would you pick it back up after failure)? The request could also time out on the client side leading the user to believe (incorrectly) that there was a failure. You might also chew up website resources (cpu, network io, etc) when you can not afford it.

There are various frameworks available that can help you with this. Check out this (IMO) very good article by Scott Hanselman titled How to run Background Tasks in ASP.NET. He iterates over various existing frameworks that would allow you do to this and even schedule the work to be done in the future if you wish.

My recommendation though is to always offload processes like this to a windows service (or other service) and not execute them on the web request (by that I mean on the web site process/application pool).


Edit based on your last edit

I know its not what you want to hear (sorry) but I do not think this will change this answer or the other given answers. ASP.NET (including MVC as it sits on top of asp.net) was just not designed with the goal of executing long running requests (like building and sending 5000 emails). You also mention logging if it fails, with a long running request like this there is no guarantee that logging will occur either. An app pool recycle is the equivalent of Process.Kill(), even if you have cleanup code and logging code there is no guarantee that it would execute. You might never know if there was a failure.

Igor
  • 60,821
  • 10
  • 100
  • 175
2

One solution to this problem is to asynchronously fire all of your async email requests, and put those tasks into a collection, then await the collection of tasks. This will probably be a bit faster since each request won't have to wait for the email request before it to complete, instead they can all happen concurrently.

Example:

var myTasks = new List<Task>();
myTasks.Add(UserManager.SendEmailAsync(userId, "Test subject line", "Sample email body"));
myTasks.Add(UserManager.SendEmailAsync(userId, "Test subject line", "Sample email body"));
myTasks.Add(UserManager.SendEmailAsync(userId, "Test subject line", "Sample email body"));
await Task.WhenAll(myTasks);

If you want a full blown fire and forget, which is probably going to be risky no matter how you slice it, check out this post by Stephen Cleary on using QueueBackgroundWorkItem. I would try out putting all of the tasks into a collection though and awaiting them to judge performance before going nuclear option.

davidallyoung
  • 1,302
  • 11
  • 15
  • My concern with this approach is that sometimes I have to send out as many as 5000 emails (per bulk). My original plan was to go with the full blown "fire and forget", but inside the "fire and forget" method, I will have a `try`...`catch` and log any error. – C.J. Mar 28 '16 at 18:31
  • In another word, I wouldn't care much even if there is any error. We want the process to move on even if there is an error. – C.J. Mar 28 '16 at 18:32
  • @C.J. I would consider the QueueBackgroundWorkItem then, which is explained in the linked article. Have you attempted this method though with an await Task.WhenAll() to know that you have a performance bottleneck? Do you have any other options such as a distributed system? I would really not want to fire and forget 5K emails, when you could have another 5K requests that all need to fire and forget 5K emails. – davidallyoung Mar 28 '16 at 18:44
  • I tried the approach mentioned in the article but I'm getting another error. It says that "DbContext has already been disposed". I believe this has something to do with out of the box `UserManager` and `SendEmailAsync` which doesn't support the background process. – C.J. Mar 28 '16 at 19:05
  • @C.J. yeah, that would be expected honestly, since once your request goes out of scope the DB Context is going to be disposed. Have you been able to test with data to scale using Task.WhenAll() to know that it will be a performance block? A distributed system approach is honestly your best bet here. – davidallyoung Mar 28 '16 at 19:21
  • WhenAll() is much better than running all 5000 tasks synchronously, but definitely not as fast as the "fire and forget" approach. – C.J. Mar 28 '16 at 19:26