1
\$\begingroup\$

I am currently developing a Task Scheduler to run different methods at a certain time. In general it is a Spring-boot application an task execution/scheduling is just a very tiny piece of the whole project. My idea is to define several Runnables that execute depending on a CronTrigger. In the Runnable I create an object Job that holds information about the current task, like startTime, status and others.

public class Job {
    private Long          id;
    private Class         clazz;
    private Date          startTime;
    private boolean       status;
    private String        parameter;
    private CronTrigger   cronTrigger;
    private String        exception;

    Job(long id, Class<?> clazz, boolean status, Date startTime, String parameter, CronTrigger cronTrigger, String exception) {
        this.id = id;
        this.clazz = clazz;
        this.startTime = startTime;
        this.status = status;
        this.parameter = parameter;
        this.cronTrigger = cronTrigger;
        this.exception = exception;
    }

    //Getter, Setter, toString()
}

To represent which thread is currently running/waiting I add it to ConcurrentHashMap<Long, Job> there are two, one for sleeping Thread and one for running Threads. There are also two methods addTask(Long task, Job job) and removeTask(Long task, Job job) that put/remove Jobs to/from the two ConcurrentHashMaps and set status of an Job.

private ThreadPoolTaskScheduler scheduler = new ThreadPoolTaskScheduler();

private HashMap<Long, CronTrigger> cronTriggers = new HashMap<>();

private ScheduledFuture<?> task0;
private ScheduledFuture<?> task1;
private ScheduledFuture<?> task2;
private ScheduledFuture<?> task3;
private ScheduledFuture<?> task4;
private ScheduledFuture<?> task5;
private ScheduledFuture<?> task6;
private ScheduledFuture<?> task7;
private ScheduledFuture<?> task8;
private ScheduledFuture<?> task9;

private final ConcurrentHashMap<Long, Job> scheduledTasksRunning = new ConcurrentHashMap<>();
private final ConcurrentHashMap<Long, Job> scheduledTasksSleeping = new ConcurrentHashMap<>();

private Job selectedScheduledJob;

private int POOL_SIZE = 10;

@Bean
public ThreadPoolTaskScheduler taskScheduler() {

    this.scheduler.setPoolSize(POOL_SIZE);
    this.scheduler.initialize();

    for (int i = 0; i < POOL_SIZE; i++){
        cronTriggers.put(((long) i), new CronTrigger("1 1 1 1 * *"));
    }

    for (int i = 0; i < POOL_SIZE; i++){
        Job job = new Job((long) i, getClass(), false, new Date(),null, cronTriggers.get((long) i), null);
        scheduledTasksSleeping.put(((long) i), job);
    }

    task0 = scheduler.schedule(new Task0(), cronTriggers.get(0L));
    task1 = scheduler.schedule(new Task1(), cronTriggers.get(1L));
    task2 = scheduler.schedule(new Task2(), cronTriggers.get(2L));
    task3 = scheduler.schedule(new Task3(), cronTriggers.get(3L));
    task4 = scheduler.schedule(new Task4(), cronTriggers.get(4L));
    task5 = scheduler.schedule(new Task5(), cronTriggers.get(5L));
    task6 = scheduler.schedule(new Task6(), cronTriggers.get(6L));
    task7 = scheduler.schedule(new Task7(), cronTriggers.get(7L));
    task8 = scheduler.schedule(new Task8(), cronTriggers.get(8L));
    task9 = scheduler.schedule(new Task9(), cronTriggers.get(9L));
    return scheduler;
}

public class Task0 implements Runnable {
    public void run() {
        Long id = 0L;
        Job job = new Job(id, getClass(), true, new Date(), null, cronTriggers.get(id), null);
        addTask(id, job);
        try {
            Thread.sleep(10000);
        } catch (InterruptedException e) {
            job.setException(e.toString());
            e.printStackTrace();
        }
        removeTask(id, job);
    }
}

public class Task1 implements Runnable {
    public void run() {
        Long id = 1L;
        Job job = new Job(id, getClass(), true, new Date(), null, cronTriggers.get(id), null);
        addTask(id, job);
        try {
            Thread.sleep(10000);
        } catch (InterruptedException e) {
            job.setException(e.toString());
            e.printStackTrace();
        }
        removeTask(id, job);
    }
}

// 8 other Runnables


/**
 * manually stop a running Job via GUI
 *
 */
public void taskCancel(){
    Long id = selectedScheduledJob.getId();
    switch (toIntExact(id)){
    case 0:
        task0.cancel(true);
        break;
    case 1:
        task1.cancel(true);
        break;
    // 8 other cases for each Thread
    }
}

/**
 * manually start a selected Job via GUI and force it to trigger at next
 * scheduled time
 */
public void taskStart(){
    Long id = selectedScheduledJob.getId();
    switch (toIntExact(id)){

    case 0:
        scheduler.schedule(new Task0(), new Date());
        scheduler.schedule(new Task0(), cronTriggers.get(id));
        break;
    case 1:
        scheduler.schedule(new Task1(), new Date());
        scheduler.schedule(new Task1(), cronTriggers.get(id));
        break;
    // 8 other cases for each Thread
    }
}

private synchronized void addTask(Long task, Job job) {
    scheduledTasksSleeping.remove(task);
    scheduledTasksRunning.put(task, job);
}

private synchronized void removeTask(Long task, Job job) {
    job.setStatus(false);
    scheduledTasksRunning.remove(task);
    scheduledTasksSleeping.put(task, job);
}

// Getter and Setter for GUI

I know however that the definition of ScheduledFuture<?> taskX; is not that good and I tend to use a ArrayList in future.

Some things I am interested in:

  • Is there anything I did that is blatantly the wrong way of doing it?
  • Do you see any way to improve the performance or speed?
  • Is it thread-safe?
\$\endgroup\$
0
\$\begingroup\$

You've already mentioned your definition of the ScheduledFuture<?> instances could be improved with an ArrayList.

for (long i=0; i<9; i++) {
    tasks[i] =  scheduler.schedule(new Task(i), cronTriggers.get(i));
}

Instead of 10 class definitions for the tasks you could set the id for your task in the constructor as you define the array of tasks. This will make your code more DRY and easier to maintain.

public class Task implements Runnable {    
    private long id;
    public Task(long id) {
        this.id = id;
    }

    public void run() {
        Job job = new Job(
            id, getClass(), true, new Date(), null, cronTriggers.get(this.id), null);
        addTask(this.id, job);
        try {
            Thread.sleep(10000);
        } catch (InterruptedException e) {
            job.setException(e.toString());
            e.printStackTrace();
        }
        removeTask(this.id, job);
    }
}
\$\endgroup\$
  • \$\begingroup\$ this is a smart improvement. Thanks \$\endgroup\$ – WirJun Jan 31 at 8:12

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.