patternjavaMinor
General Batched Job Runner
Viewed 0 times
runnerjobbatchedgeneral
Problem
This question is inspired by:
Generic Task Scheduler
where the problem is to run tasks on a scheduled basis, in parallel, and have individual timeouts for each job.
For example, consider this use-case:
Which sets up a supplier, and a completion consumer. The supplier is called every 10 seconds, and the individual jobs are expected to be complete within 5 seconds. The completion consumer will be called for each task when the task completes, or it has been running for 5 seconds, which ever comes first. If the timeout happens first, the
Note that even though the above code uses Java8 Functional interfaces, the core class is fully Java7 compatible.
I'm looking for reviews of the concurrency model, any issues you may see at all.
One issue I am aware of is that the system requires at least one listener thread for each task. Since the listener threads do nothing other than wait, they are low-impact, but, there may be a lot of them. There may be the need for a 'throttle' on the number of active tasks.
The code is intended to be compatible with Java7.
```
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Condi
Generic Task Scheduler
where the problem is to run tasks on a scheduled basis, in parallel, and have individual timeouts for each job.
For example, consider this use-case:
BatchedJobHandler.TaskSupplier supplier =
() -> getTasksForExecution();
BatchedJobHandler.TaskCompletion completion =
(t, b, f) -> taskAvailable(t, b, f);
BatchedJobHandler batcher = new BatchedJobHandler<>(
supplier, completion, 10, 5, TimeUnit.SECONDS);
batcher.start();Which sets up a supplier, and a completion consumer. The supplier is called every 10 seconds, and the individual jobs are expected to be complete within 5 seconds. The completion consumer will be called for each task when the task completes, or it has been running for 5 seconds, which ever comes first. If the timeout happens first, the
timedout value will be true.Note that even though the above code uses Java8 Functional interfaces, the core class is fully Java7 compatible.
I'm looking for reviews of the concurrency model, any issues you may see at all.
One issue I am aware of is that the system requires at least one listener thread for each task. Since the listener threads do nothing other than wait, they are low-impact, but, there may be a lot of them. There may be the need for a 'throttle' on the number of active tasks.
The code is intended to be compatible with Java7.
```
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Condi
Solution
I can't find anything wrong with this.
Just ... nitpicks.
You name your variables in for each loops
I think if you gave them a better name like
Your test code seems to be on emergency rationing for variable name characters.
Lastly, you should keep camelCasing in mind for your variables.
These two variables don't seem to follow camelCasing.
Bad rolfl. This is the kind of comment that is written once and then dies a slow death. If you must leave such comments, name it
... and if a TODO stays there for a very long time, maybe the cheat way isn't so bad.
Just ... nitpicks.
You name your variables in for each loops
t.for (T t : supplier.tasks()) {
taskQueue.add(t);
}I think if you gave them a better name like
task, it would improve readability.Your test code seems to be on emergency rationing for variable name characters.
private static Iterable getTasksForExecution() {
int cnt = 5 + rand.nextInt(10);
System.out.println("Getting " + cnt + " tasks");
List ret = new ArrayList<>(cnt);
for (int i = 0; i < cnt; i++) {
ret.add(new SleepTask(idgen.incrementAndGet(), 2000 + rand
.nextInt(5000)));
}
return ret;
}cnt should be count (and maybe even taskCount). ret... personally I like result for that.Lastly, you should keep camelCasing in mind for your variables.
private long delayto = 0L;
private final LinkedBlockingQueue logqueue = LOG_ENABLED ? new LinkedBlockingQueue<>()
: null;These two variables don't seem to follow camelCasing.
/*
* Cheat way to do logging.... Replace with real logging later.
*/Bad rolfl. This is the kind of comment that is written once and then dies a slow death. If you must leave such comments, name it
TODO somewhere. Even if it slips through the cracks of your issue management system, you'll still be able to search for TODO and find the remaining tasks.... and if a TODO stays there for a very long time, maybe the cheat way isn't so bad.
Code Snippets
for (T t : supplier.tasks()) {
taskQueue.add(t);
}private static Iterable<SleepTask> getTasksForExecution() {
int cnt = 5 + rand.nextInt(10);
System.out.println("Getting " + cnt + " tasks");
List<SleepTask> ret = new ArrayList<>(cnt);
for (int i = 0; i < cnt; i++) {
ret.add(new SleepTask(idgen.incrementAndGet(), 2000 + rand
.nextInt(5000)));
}
return ret;
}private long delayto = 0L;
private final LinkedBlockingQueue<String> logqueue = LOG_ENABLED ? new LinkedBlockingQueue<>()
: null;/*
* Cheat way to do logging.... Replace with real logging later.
*/Context
StackExchange Code Review Q#68080, answer score: 3
Revisions (0)
No revisions yet.