views:

470

answers:

5

Sorry the title is a bit crappy, I couldn't quite word it properly.

Edit: I should note this is a console c# app

I've prototyped out a system that works like so (this is rough pseudo-codeish):

var collection = grabfromdb();

foreach (item in collection) {
    SendAnEmail();
}

SendAnEmail:

SmtpClient mailClient = new SmtpClient;
mailClient.SendCompleted += new SendCompletedEventHandler(SendComplete);
mailClient.SendAsync('the mail message');

SendComplete:

if (anyErrors) {
    errorHandling()
}
else {
    HitDBAndMarkAsSendOK();    
}

Obviously this setup is not ideal. If the initial collection has, say 10,000 records, then it's going to new up 10,000 instances of smtpclient in fairly short order as quickly as it can step through the rows - and likely asplode in the process.

My ideal end game is to have something like 10 concurrent email going out at once.

A hacky solution comes to mind: Add a counter, that increments when SendAnEmail() is called, and decrements when SendComplete is sent. Before SendAnEmail() is called in the initial loop, check the counter, if it's too high, then sleep for a small period of time and then check it again.

I'm not sure that's such a great idea, and figure the SO hive mind would have a way to do this properly.

I have very little knowledge of threading and not sure if it would be an appropriate use here. Eg sending email in a background thread, first check the number of child threads to ensure there's not too many being used. Or if there is some type of 'thread throttling' built in.


Update

Following in the advice of Steven A. Lowe, I now have:

  • A Dictionary holding my emails and a unique key (this is the email que
  • A FillQue Method, which populates the dictionary
  • A ProcessQue method, which is a background thread. It checks the que, and SendAsycs any email in the que.
  • A SendCompleted delegate which removes the email from the que. And calls FillQue again.

I've a few problems with this setup. I think I've missed the boat with the background thread, should I be spawning one of these for each item in the dictionary? How can I get the thread to 'hang around' for lack of a better word, if the email que empties the thread ends.


final update

I've put a 'while(true) {}' in the background thread. If the que is empty, it waits a few seconds and tries again. If the que is repeatedly empty, i 'break' the while, and the program ends... Works fine. I'm a bit worried about the 'while(true)' business though..

+1  A: 

I believe your hacky solution actually would work. Just make sure you have a lock statement around the bits where you increment and decrement the counter:

class EmailSender
{
  object SimultaneousEmailsLock;
  int SimultaneousEmails;
  public string[] Recipients;

  void SendAll()
  {
    foreach(string Recipient in Recipients)
    {
      while (SimultaneousEmails>10) Thread.Sleep(10);
      SendAnEmail(Recipient);
    }
  }

  void SendAnEmail(string Recipient)
  {
    lock(SimultaneousEmailsLock)
    {
      SimultaneousEmails++;
    }

    ... send it ...
  }

  void FinishedEmailCallback()
  {
    lock(SimultaneousEmailsLock)
    {
      SimultaneousEmails--;
    }

    ... etc ...
  }
}
Chris
A: 

You could use a .NET Timer to setup the schedule for sending messages. Whenever the timer fires, grab the next 10 messages and send them all, and repeat. Or if you want a general (10 messages per second) rate you could have the timer fire every 100ms, and send a single message every time.

If you need more advanced scheduling, you could look at a scheduling framework like Quartz.NET

Andy White
Thanks for the response, the other side effect of this approach is that I can easily limit the number of results pulled back by the DB to only grab 10 at a time.
DaRKoN_
A: 

Isn't this something that Thread.Sleep() can handle?

You are correct in thinking that background threading can serve a good purpose here. Basically what you want to do is create a background thread for this process, let it run its own way, delays and all, and then terminate the thread when the process is done, or leave it on indefinitely (turning it into a Windows Service or something similar will be a good idea).

A little intro on multi-threading can be read here (with Thread.Sleep included!).

A nice intro on Windows Services can be read here.

Jon Limjap
+2  A: 

Short Answer

Use a queue as a finite buffer, processed by its own thread.

Long Answer

Call a fill-queue method to create a queue of emails, limited to (say) 10. Fill it with the first 10 unsent emails. Launch a thread to process the queue - for each email in the queue, send it asynch. When the queue is empty sleep for a while and check again. Have the completion delegate remove the sent or errored email from the queue and update the database, then call the fill-queue method to read more unsent emails into the queue (back up to the limit).

You'll only need locks around the queue operations, and will only have to manage (directly) the one thread to process the queue. You will never have more than N+1 threads active at once, where N is the queue limit.

Steven A. Lowe
This sounds like it. Could you possibly flesh this answer out with a brief code example? What would be an ideal type to use for the que (IList?)?
DaRKoN_
@[DaRKoN_]: yes, I'd probably use an IList<item> or Queue<item> for whatever "item" you used in your "collection" in the original question, i.e. whatever info you need to create and send the email - custom class, datarow, whatever.
Steven A. Lowe
+1  A: 

I would add all my messages to a Queue, and then spawn i.e. 10 threads which sent emails until the Queue was empty. Pseudo'ish C# (probably wont compile):

class EmailSender
{
    Queue<Message> messages;
    List<Thread> threads;

    public Send(IEnumerable<Message> messages, int threads)
    {
        this.messages = new Queue<Message>(messages);
        this.threads = new List<Thread>();
        while(threads-- > 0)
            threads.Add(new Thread(SendMessages));

        threads.ForEach(t => t.Start());

        while(threads.Any(t => t.IsAlive))
            Thread.Sleep(50);
    }

    private SendMessages()
    {
        while(true)
        {
            Message m;
            lock(messages)
            {
                try
                {
                    m = messages.Dequeue();
                }
                catch(InvalidOperationException)
                {
                    // No more messages
                    return;
                }
            }

            // Send message in some way. Not in an async way, 
            // since we are already kind of async.

            Thread.Sleep(); // Perhaps take a quick rest
        }
    }
}

If the message is the same, and just having many recipients, just swap the Message with a Recipient, and add a single Message parameter to the Send method.

Svish
Even with the use of a semaphore to limit to n active workers, you don't want to spawn 10000 tasks which then just wait. This with each worker being long(ish) running means the "dedicated thread pool" makes sense.
Richard
huh? spawn 10000 tasks? what are you talking about?
Svish