4

I need to send a email during registration process , so for this reason i am using Java Mail API , this is working fine , but observed that the email process is taking nearly 6 seconds (which is too long ) so Ajax call making the user wait too long for response

so for this reason i have decided to use background thread for sending email so the user need not wait for the Ajax call response (Jersey REST Web Service call)

My question is it a good practice to creating threads in a webapplication for every request ??

@Path("/insertOrUpdateUser")
public class InsertOrUpdateUser {
        final static Logger logger = Logger.getLogger(InsertOrUpdateUser.class);
        @GET
        @Consumes("application/text")
        @Produces("application/json")
        public String getSalesUserData(@QueryParam(value = "empId") String empId
                        )
                        throws JSONException, SQLException {
                JSONObject final_jsonobject = new JSONObject();
            ExecutorService executorService = Executors.newFixedThreadPool(10);
                                executorService.execute(new Runnable() {
                                 public void run() {
                                         try {
                                                        SendEmailUtility.sendmail(emaildummy);
                                                } catch (IOException e) {
                                                        logger.error("failed",e);

                                                }
                                 }
                              });

                        }


                } catch (SQLException e) {

                } catch (Exception e) {


                }


                   finally {

                        }


                return response;
        }




}

And this is my Utility class for sending email

public class SendEmailUtility
{
    public static String sendmail(String sendto)
        throws IOException
    {
        String result = "fail";
        Properties props_load = getProperties();
        final String username = props_load.getProperty("username");
        final String password = props_load.getProperty("password");
        Properties props_send = new Properties();
        props_send.put("mail.smtp.auth", "true");
        props_send.put("mail.smtp.starttls.enable", "true");
        props_send.put("mail.smtp.host", props_load.getProperty("mail.smtp.host"));
        props_send.put("mail.smtp.port", props_load.getProperty("mail.smtp.port"));

        Session session = Session.getInstance(props_send,
            new javax.mail.Authenticator() {
                @Override
                protected PasswordAuthentication getPasswordAuthentication()
                {
                    return new PasswordAuthentication(username, password);
                }
            });

        try {
            Message message = new MimeMessage(session);
            message.setFrom(new InternetAddress(props_load.getProperty("setFrom")));
            message.setRecipients(Message.RecipientType.TO, InternetAddress.parse(sendto));
            message.setText("Some Text to be send in mail");
            Transport.send(message);
            result = "success";
        } catch (MessagingException e) {
            result = "fail";
            logger.error("Exception Occured - sendto: " + sendto, e);
        }
        return result;
    }
}

Could you please let me know if this is best practice to do in a web application ??

Pawan
  • 28,159
  • 84
  • 232
  • 394
  • The email is sent server-side isn't it? So it doesn't matter in which thread you run it as it will always take the same amount of time. Maybe you want to show a circular intermediate progress bar to the user indicating that his request can take some seconds to complete. – xdevs23 Sep 06 '16 at 15:46
  • Have you considered your internet speed also? – Enzokie Sep 06 '16 at 15:47
  • Sorry for the confusion created , with the help of background thread the Ajax call is not waiting for the email process , so its fine with respect to ajax resonse , but my concern is related to number of threads that are being created . – Pawan Sep 06 '16 at 15:49
  • Your example doesn't show creating a new thread for every request. It shows submitting tasks to a threadpool, which is a much better plan. – Nathan Hughes Sep 06 '16 at 15:50
  • @NathanHughes , so this below line isn't a problem ?? ExecutorService executorService = Executors.newFixedThreadPool(10); – Pawan Sep 06 '16 at 15:52
  • Tasks that take a long time and do not require user intervention should be handed off to a message queue, either internal to your application or external. – Sean Bright Sep 06 '16 at 15:52
  • Creating a threadpool for every request would be silly. hard to tell what is intended when you have snippets like this. also @Sean has a point that queueing up these email requests would be a good idea. – Nathan Hughes Sep 06 '16 at 15:53
  • `@Path("/insertOrUpdateUser")` - I hate to be that guy… but this is not very RESTful. – Sean Bright Sep 06 '16 at 15:56

3 Answers3

8

There are host of ways you can handle it, so it all depends on whether your application server has that much resources (memory, threads etc.) to handle your implementation, so it makes you best person to decide on which approach to go.

As such it is not bad practice to spawn parallel threads for doing something if it is justified by design, but typically you should go with controlled threads.

Please note that whether you use newSingleThreadExecutor() or newFixedThreadPool(nThreads), under-the-hoods there will always be a ThreadPoolExecutor object created.

My recommendation will be to use seconds option in below list i.e. "Controlled number of threads", and in that specify max thread count as you see fir.

One thread for each request

In this approach one thread will be created for each incoming request from GUI, so if you are getting 10 requests for inserting/updating user then 10 threads will be spawned which will send emails.

Downside of this approach is that there is no control on number of threads so you can end with StackOverflowException or may be memory issue.

Please make sure to shutdown your executor service else you will end up wasting JVM resources.

// inside your getSalesUserData() method
ExecutorService emailExecutor = Executors.newSingleThreadExecutor();
        emailExecutor.execute(new Runnable() {
            @Override
            public void run() {
                try {
                    SendEmailUtility.sendmail(emaildummy);
                } catch (IOException e) {
                    logger.error("failed", e);
                }
            }
        });
        emailExecutor.shutdown(); // it is very important to shutdown your non-singleton ExecutorService.

Controlled number of threads

In this approach, some pre-defined number of threads will be present and those will process your email sending requirement. In below example I am starting a thread pool with max of 10 threads, then I am using a LinkedBlockingQueue implementation so this will ensure that if there are more than 10 requests and currently all my 10 threads are busy then excess of requests will be queued and not lost, this is the advantage you get with LinkedBlockingQueue implementation of Queue.

You can initialize you singleton ThreadPoolExecutor upon application server start, if there are no requests then no threads will be present so it is safe to do so. In fact I use similar configuration for my prod application.

I am using time to live seconds as 1 seconds so if a thread is ideal in JVM for more than 1 seconds then it will die.

Please note that since same thread pool is used for processing all you requests, so it should be singleton and do not shutdown this thread pool else your tasks will never be executed.

// creating a thread pool with 10 threads, max alive time is 1 seconds, and linked blocking queue for unlimited queuing of requests.
        // if you want to process with 100 threads then replace both instances of 10 with 100, rest can remain same...
        // this should be a singleton
        ThreadPoolExecutor executor = new ThreadPoolExecutor(10, 10, 1, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>());

        // inside your getSalesUserData() method
        executor.execute(new Runnable() {
            @Override
            public void run() {
                try {
                    SendEmailUtility.sendmail(emaildummy);
                } catch (IOException e) {
                    logger.error("failed", e);
                }
            }
        });

Java's default cached thread pool

This approach is much like above, only that Java will initialize the ThreadPoolExecutor for you as ThreadPoolExecutor(0, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, new SynchronousQueue<Runnable>());

Here max number of threads will be Integer.MAX_VALUE, so threads will be created as needed and time to live will be 60 seconds.

If you want to use this way then below is the way.

// this should be a singleton
        ExecutorService emailExecutor = Executors.newCachedThreadPool();

        // from you getSalesUserData() method
        emailExecutor.execute(new Runnable() {
            @Override
            public void run() {
                try {
                    SendEmailUtility.sendmail(emaildummy);
                } catch (IOException e) {
                    logger.error("failed", e);
                }
            }
        });
hagrawal
  • 12,025
  • 4
  • 33
  • 61
  • in the last option, should ExecutorService also be created using Singleton concept or is it okay to create emailExecutor for each incoming request? – zee Jan 26 '17 at 08:15
  • @zee I have mentioned in my code -`// this should be a singleton`. If you do are executing `ExecutorService emailExecutor = Executors.newCachedThreadPool();` for each request then essentially you are get a new thread pool created for each request. – hagrawal Jan 27 '17 at 07:16
  • How can you test the code with Java's default cached thread pool (like Unit Testing) ? – Reneta Jun 03 '20 at 11:59
  • I put the thread to sleep for 100ms: `Thread.sleep(100);`` and the test pass, because it had the time and the mail is sent, I guess it is not a bad way to test it. – Reneta Jun 04 '20 at 06:42
2

Manually creating of ExecutorService on java web serer is bad idea. In your implementation for each request you create 10 threads.

Better solution is to use ManagedExecutorService (example) if you work with JEE7 or ThreadPoolTaskExecutor if you work with Spring(docs).

If you work with Tomcat you should read this thread.

Community
  • 1
  • 1
Koziołek
  • 2,570
  • 25
  • 37
  • i am using tomcat , thank you very much , i can use for my starting point for this task to be done . – Pawan Sep 06 '16 at 16:09
1

The best practice is to use a single ExecutorService to provide a thread pool for all requests. You probably want to configure the ExecutorService with a non-zero, but limited, number of threads.

The idea here is that you will have some threads that are reused throughout the lifetime of the application. You get the added benefit that if there is a temporary slowdown (or halt) in sending emails, you don't end up with a growing number of threads Instead, you end up with a growing number of pieces of work (emails to send) to be executed, which is much less resource intensive than extra threads.

Rob
  • 5,634
  • 1
  • 21
  • 28
  • this seems useful related to my question , is this possible to configure ThreadPool for my case ?? – Pawan Sep 06 '16 at 15:58
  • 1
    Yes, just make your ExecutorService static at the class level (like your logger) and it will be created once when the class is loaded and live until the application is stopped. You can implement a `ServletContextListener` to explicitly manage the lifecycle and ensure proper shutdown and cleanup (e.g., so your application will wait until all messages have been sent before shutting down). – Rob Sep 06 '16 at 16:02