HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Concurrent task processing queue

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
processingconcurrentqueuetask

Problem

I can have a maximum of three concurrent threads processing tasks. Each of these threads can process 1 to 100 tasks simultaneously (by connecting to an external system). The time taken to process 100 tasks at once in one thread is the same as it takes to process 1 task in one thread because the overhead of connecting to the external system is what takes 95% of the time. Tasks come in at random intervals from other threads in the application, these threads need to block until the task is done, or a timeout is hit. Responding quickly to the threads submitting tasks is the primary goal here, and making use of the ability to process tasks in batch is secondary (but useful and important when we have a big queue of tasks as it saves time).

This code works (or appears to anyway), but before I build it into my larger application I want to check that this can't be done better/faster etc. It has the potential to create a major bottleneck in the application so I want to make sure it is done in the most efficient way possible. I'm not very experienced with the concurrent package. Any feedback would be greatly appreciated.

Here is the concept code that I have written to test with. System outs will be replaced with logging when it is built into the main application.

```
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.Semaphore;

public class TaskProcessor implements Runnable
{
private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>();

private final Semaphore semaphore = new Semaphore(3);

private final Executor executor = Executors.newCachedThreadPool();

public static void main(final String[] args)
{
final TaskProcessor taskProcessor = new TaskProcessor();
new Thread(taskProcessor).start();
for (int i = 0; i tasks = new ArrayList<>();

System.out.pr

Solution

I am not a Java expert, but

  • In your description you talk about tasks and jobs, however your code is only using task, which makes your code harder to follow.



  • If you are going to log "waiting for task" then you should also log "waiting for semaphore"



  • Related to that, I have a suspicion that your semaphore approach does not work, you should try to run more than 3 tasks.



  • I would allow the caller to pass 3 and 100 as parameters or read this from a config file or environment variables

Context

StackExchange Code Review Q#47352, answer score: 3

Revisions (0)

No revisions yet.