views:

122

answers:

5

I have a very complex system (100+ threads) which need to send email without blocking. My solution to the problem was to implement a class called EmailQueueSender which is started at the beginning of execution and has a ScheduledExecutorService which looks at an internal queue every 500ms and if size()>0 it empties it.

While this is going on there's a synchronized static method called addEmailToQueue(String[]) which accepts an email containing body,subject..etc as an array. The system does work, and my other threads can move on after adding their email to queue without blocking or even worrying if the email was successfully sent...it just seems to be a little messy...or hackish...Every programmer gets this feeling in their stomach when they know they're doing something wrong or there's a better way. That said, can someone slap me on the wrist and suggest a more efficient way to accomplish this?

Thanks!

+4  A: 

http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ThreadPoolExecutor.html

this class alone will probably handle most of the stuff you need. just put the sending code in a runnable and add it with the execute method. the getQueue method will allow you to retrieve the current list of waiting items so you can save it when restarting the sender service without losing emails

Lorenzo Boccaccia
+2  A: 

If you are using Java 6, then you can make heavy use of the primitives in the java.util.concurrent package.

Having a separate thread that handles the real sending is completely normal. Instead of polling a queue, I would rather use a BlockingQueue as you can use a blocking take() instead of busy-waiting.

If you are interested in whether the e-mail was successfully sent, your append method could return a Future so that you can pass the return value on once you have sent the message.

Instead of having an array of Strings, I would recommend creating a (almost trivial) Java class to hold the values. Object creation is cheap these days.

andri
Just what I needed! Already implemented and working perfectly. I also changed the email to an internal class rather than an array....Thank you so much :)
Submerged
j.u.c came out in java 5
John V.
A: 

There might be a full blown mail package out there already, but I would probably start with Spring's support for email and job scheduling. Fire a new job for each email to be sent, and let the timing of the executor send the jobs and worry about how many need to be done. No queuing involved.

Underneath the framework, Spring is using Java Mail for the email part, and lets you choose between ThreadPoolExecutor (as mention by @Lorenzo) or Quartz. Quartz is better in my opinion, because you can even set it up so that it fires your jobs at fixed points in time like cron jobs (eg. at midnight). The advantage of using Spring is that it greatly simplifies working with these packages, so that your job is even easier.

harschware
A: 

There are many packages and tools that will help with this, but the generic name for cases like this, extensively studied in computer science, is producer-consumer problem. There are various well-known solutions for it, which could be considered 'design patterns'.

DJClayworth
+1  A: 

Im not sure if this would work for your application, but sounds like it would. A ThreadPoolExecutor (an ExecutorService-implementation) can take a BlockingQueue as argument, and you can simply add new threads to the queue. When you are done you simply terminate the ThreadPoolExecutor.

private BlockingQueue<Runnable> queue; 
... 
ThreadPoolExecutor executor = new ThreadPoolExecutor(10, 10, new Long(1000),  
                TimeUnit.MILLISECONDS, this.queue);

You can keep a count of all the threads added to the queue. When you think you are done (the queue is empty, perhaps?) simply compare this to

 if (issuedThreads == pool.getCompletedTaskCount()) { 
        pool.shutdown(); 
    } 

If the two match, you are done. Another way to terminate the pool is to wait a second in a loop:

try { 
      while (!this.pool.awaitTermination(1000, TimeUnit.MILLISECONDS)); 
} catch (InterruptedException e) {//log exception...} 
Lars Andren