Implementation of a Thread queue

[salvin francis]

I have a webbased application and I need to improve the response time of a few requests coming in. There are a few tasks performed like audits which may be processed in parallel.
I was thinking for a thread queue for this to work, rather than creating one my self, I was thinking of using ThreadPoolExecutor :

public class SomeController extends MultiActionController {
	ThreadPoolExecutor threadPoolExecutor;
	
	public void saveData(HttpServletRequest request, HttpServletResponse response){
	
		//-- DB Save logic comes here...
	
		//-- The other stuff will run in parallel so this method will immediately return here.
		if(this.threadPoolExecutor == null){
			//-- Initialize only once..
			this.threadPoolExecutor = new ThreadPoolExecutor(1, 10, 20, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>(20));
		}
		this.threadPoolExecutor.execute(new Runnable() {
			@Override
			public void run() {
				//-- Code to run in parallel comes here...
				
			}
		});
	}
}

Is this a good way to implement it ?

[salvin francis]

Made it a bit more safer:

public class SomeController extends MultiActionController {
	ThreadPoolExecutor threadPoolExecutor;
	
	public void saveData(HttpServletRequest request, HttpServletResponse response){
	
		//-- DB Save logic comes here...
	
		//-- The other stuff will run in parallel so this method will immediately return here.
		//-- Made it a bit safer with synchronized block
		synchronized(this){
			if(this.threadPoolExecutor == null){
				//-- Initialize only once..
				this.threadPoolExecutor = new ThreadPoolExecutor(1, 10, 20, TimeUnit.SECONDS, new ArrayBlockingQueue<Runnable>(20));
			}
			this.threadPoolExecutor.execute(new Runnable() {
				@Override
				public void run() {
					//-- Code to run in parallel comes here...
					
				}
			});
		}
	}
}

[salvin francis]

Here is another implementation that came to my mind:

public class SomeController extends MultiActionController {
	private ExecutorService executorService;
	
	public void saveData(HttpServletRequest request, HttpServletResponse response){
	
		//-- DB Save logic comes here...
	
		//-- The other stuff will run in parallel so this method will immediately return here.
		
		//-- Hopefully, this synchronized block shouldn't affect performance much :(
		synchronized(this){
			if(this.executorService == null){
				//-- Initialize only once..
				this.executorService = Executors.newCachedThreadPool();
			}					
		}
		this.executorService.execute(new Runnable() {
			@Override
			public void run() {
				//-- Do your other stuff here 
		
			}
		});
	}
}

[Jesper de Jong]

Are you using a web app framework? What is MultiActionController?

If you would do this in a servlet, then it could be a problem. The servlet container manages creating instances of servlets, and for whatever reason it may decide to create one or more instances of a servlet. If it would create multiple instances, then each of them would have their own ExecutorService instance.

Another problem is that you'll never get a chance to shutdown the ExecutorService. If you would stop your webapp (for example in Tomcat's manager application), then the threads started by the ExecutorService would continue to exist and run. So it would be a resource leak.

[Tim Holloway]

Since we don't have all the context here, we can't offer advice on all the details, but there are 2 things you need to keep in mind:

1. You should NEVER spawn threads or thread pools from within the execute() method of a servlet, its heirs (doGet(), doPost(), etc. or any class or method invoked from a servlet. That includes JSPs and action classes for such framworks as Struts, JSF, Wicket, and so forth. If you want thread pools, create them from a ServletContextListener, where you won't pollute the request/response thread pool that's handling servlet requests.

2. DON'T attempt to save HttpServletRequest and HttpServletResponse objects outside of the execute() method of a servlet or its heirs and assigns. Those objects are only valid for the duration of the servlet request method. Same goes for other internal resources like JDBC Connection objects.

[salvin francis]

Jesper de Jong wrote:Are you using a web app framework?

Yes, Using Spring MVC

Jesper de Jong wrote:
What is MultiActionController?

http://docs.spring.io/spring-framework/docs/2.5.x/api/org/springframework/web/servlet/mvc/multiaction/MultiActionController.html

[salvin francis]

Jesper de Jong wrote:
If you would do this in a servlet, then it could be a problem. The servlet container manages creating instances of servlets, and for whatever reason it may decide to create one or more instances of a servlet. If it would create multiple instances, then each of them would have their own ExecutorService instance.

This is something I need to research on. As far as I know, a single servlet exists within spring ie the dispatcher and it delegates the incoming request to respective Controllers.
I assume that Spring will create a Singleteon object (since it does so by default for all beans) and call its method (saveData() in my case) and pass the request and response object to it.

Jesper de Jong wrote:
Another problem is that you'll never get a chance to shutdown the ExecutorService.

Ahh this is something that struck me as well... When do I actually shut it down ? I don't have an answer to that, need to research.

Jesper de Jong wrote:
If you would stop your webapp (for example in Tomcat's manager application), then the threads started by the ExecutorService would continue to exist and run. So it would be a resource leak.

Yes, but then these threads are quite shortlived, they would complete their tasks within a few secs. However, in short the answer is yes, it would be a resource leak in the event a thread is executing when the Tomcat is shutdown.

[salvin francis]

Tim Holloway wrote:Since we don't have all the context here, we can't offer advice on all the details,

Here is how multiaction controllers work : http://www.mkyong.com/spring-mvc/spring-mvc-multiactioncontroller-example/

Tim Holloway wrote:
1. You should NEVER spawn threads or thread pools from within the execute() method of a servlet, its heirs (doGet(), doPost(), etc. or any class or method invoked from a servlet. That includes JSPs and action classes for such framworks as Struts, JSF, Wicket, and so forth. If you want thread pools, create them from a ServletContextListener, where you won't pollute the request/response thread pool that's handling servlet requests.

Thats a valid point, I'll check into using ServletContextListener for this.

Tim Holloway wrote:
2. DON'T attempt to save HttpServletRequest and HttpServletResponse objects outside of the execute() method of a servlet or its heirs and assigns. Those objects are only valid for the duration of the servlet request method. Same goes for other internal resources like JDBC Connection objects.

I have analyzed this, the working here is that besides a few params sent by the client in request object, rest is provided by spring itself. I could mark the variables as final and pass them to this anonymous Runnable class.

Here is a bit more detail about my implementation plan:

public class SomeController extends MultiActionController {
    private ExecutorService executorService;
    private SomeSpringInjectedDao springInjectedDao;
	//-- setters, getters for dao
	
    public void saveData(HttpServletRequest request, HttpServletResponse response){
		final String param1 = request.getParameter("something1");
		final String param2 = request.getParameter("something2");
		final String param3 = request.getParameter("something3");
        //-- DB Save logic comes here...
		springInjectedDao.doSomeSaveAction(param1, param2, param3);
		
        synchronized(this){
            if(this.executorService == null){
                this.executorService = Executors.newCachedThreadPool();
            }                   
        }
        this.executorService.execute(new Runnable() {
            @Override
            public void run() {
				springInjectedDao.doSomeParallelAction(param1, param2, param3);
            }
        });
    }
}

[Stephan van Hulst]

salvin francis wrote:Ahh this is something that struck me as well... When do I actually shut it down ? I don't have an answer to that, need to research.

I think the safest is, before the end of a request. But since that would defeat the purpose of using them in the first place, it goes to show that maybe using an ExecutorService in this way isn't the best way to start with.

Yes, but then these threads are quite shortlived, they would complete their tasks within a few secs. However, in short the answer is yes, it would be a resource leak in the event a thread is executing when the Tomcat is shutdown.

It doesn't matter how long the tasks are, because the threads are cached and alive even after the tasks finish. That's the entire point of a thread pool. The pool will keep running indefinitely, unless you terminate it or all the threads are daemons (which is NOT what you want for transactions).

[salvin francis]

Hi Stephan,
There is there any other way to implement this ?

[Stephan van Hulst]

I'm not very experienced with Java servlets, but you might want to try injecting a ManagedExecutorService into a field using the @Resource annotation, and then execute your tasks like that. Note that even so, you should probably use the invokeAll() method so that the tasks finish running before the request finishes.

public class SomeController extends MultiActionController {

  @Resource
  private ManagedExecutorService executor;

  public void saveData(HttpServletRequest request, HttpServletResponse response) {
    try {
      executor.invokeAll(Arrays.asList(
        new SomeManagedTask(request.getParameter("something")),
        new SomeOtherManagedTask(request.getParameter("somethingElse"))
      ));
    }

    catch (InterruptedException ex) {
      doSomethingUsefulWithException(ex);
    }
  }
}

[Tim Holloway]

One of the biggest reasons that you should not spawn threads from within the servlet request/response function is that you don't own the thread that you're running under. The server pulls it from a pool, dispatches it, then returns the thread to the pool for some other request to run under. For a pool to work properly, all resources in the pool must be identical (interchangeable). Otherwise they won't operate consistently and predictably.

If you spawn a thread and it "only takes a few seconds", but its parent thread gets returned to the pool in a few milliseconds, you could potentially crash the server and/or scramble completely unrelated webapps.

You can safely construct a thread pool in a ServletContextListener's contextInitialized() method and then safely destroy it in the contextDestroyed() method.

Note that your service threads should have a way of accepting and acting on termination requests. The thread.cancel() method is not only deprecated, it's dangerous (which is why it's deprecated). And until ALL threads in the pool terminate, the entire webapp server will hang. JVMs only shut down when the last of their threads has shut down.

[salvin francis]

Interesting replies !!

Looks like for ManagedExecutorService, I would have to add the jar : javax.enterprise.concurrent-api-1.0.jar to my application. However, I would really avoid that at the moment since our application is already into production and I would really not want to experiment with new libraries at the moment.
However, if I am able to study and see a real performance boost by doing things in parallel, maybe in near future I could suggest this to our team here.


From what I understand from all posts above, It seems that there is no baked solution for this. I am thinking the old school way of doing this ...

public class SomeController extends MultiActionController {
    ThreadPoolExecutor threadPoolExecutor;
     
    public void saveData(HttpServletRequest request, HttpServletResponse response){
     
        //-- DB Save logic comes here...
     
        //-- Plain old simple vanilla threads that will execute my tasks in parallel
        new Thread(){
                @Override
                public void run() {
					//-- Task 1
                }
        }.start();
        new Thread(){
                @Override
                public void run() {
					//-- Task 2
                }
        }.start();
    }
}

[Stephan van Hulst]

And Tim explained that that is a really bad idea. Don't spawn threads or thread pools inside your servlet actions. The servlet container needs to manage all the resources, and if you work around that, things will eventually break.

Have you already tried profiling the application to see exactly which parts can use optimization?

[salvin francis]

Unfortunately, yes.
Profiling has revealed that auditing is infact a huge memory hogger for our app.
In order to create the audit trail, various queries and checks are made to see what field was modified and by whom. Since it does not play a part in the main saving logic, it was one of our candidates for sidelining. Another candidate was an indexer for search.

I am still looking into how to hook this in ServletContextListener via spring in the most efficient way.

[Stephan van Hulst]

You realize that performing more tasks in parallel is going to increase memory usage?

[Tim Holloway]

Spring cannot inject anything into the ServletContextListener, since it's instantiated by the webapp server, not Spring.

I believe, however, that you can obtain the Spring Bean factory from the Spring Application Context and use that to obtain Spring-managed resources. All that's necessary for that is that Spring initializes itself before the ServletContextListener does.

[salvin francis]

Stephan van Hulst wrote:You realize that performing more tasks in parallel is going to increase memory usage?

Yes, but its a tradeoff that a user gets a faster response when saving an entry.

Tim Holloway wrote:I believe, however, that you can obtain the Spring Bean factory from the Spring Application Context and use that to obtain Spring-managed resources.

I figure that this can be used to get a reference to a bean that's managing my threads. But, I am still a bit confused as to when do I actually shutdown the pool ?

[salvin francis]

http://www.mkyong.com/spring/spring-postconstruct-and-predestroy-example/

This example demonstrates a bean which has a construction and well as destroy method. I think I could use this.