kagkarlsson / db-scheduler

Persistent cluster-friendly scheduler for Java
Apache License 2.0
1.25k stars 191 forks source link

Register Disabled Schedules Without Exceptions When Using RecurringTaskWithPersistentSchedule #443

Open the-jbutler opened 11 months ago

the-jbutler commented 11 months ago

Prerequisites

Please answer the following questions for yourself before submitting an issue. YOU MAY DELETE THE PREREQUISITES SECTION.

Expected Behavior

While using RecurringTaskWithPersistentSchedule it would be useful to have some mechanism to pass in a disabled schedule when scheduling tasks. When specifying a "disabled" schedule during creation of a recurring task it should either not register the task at all, or write the task to the database but never attempt to execute it, updating or deleting any existing instance of the task that have been disabled to prevent their execution too.

Current Behavior

Currently an exception is thrown during startup as CronSchedule attempts to calculate the next execution date for a disabled schedule. This is currently expressing itself in my integration tests as I have some testing to ensure the config driven system implemented will disable tasks that we don't want running on a schedule. But it will be a problem for a real deployment in time.

Looking through the code I can see that the disabled state of a schedule is considered where a normal RecurringTask is registered using RecurringTask#onStartup, however recurring tasks using RecurringTaskWithPersistentSchedule aren't registered in this way looking at the provided examples.

Example

I am trying to implement a Spring Boot configuration driven method of registering arbitrary numbers of scheduled tasks at startup. The tasks all perform the same action, however they are to run at different schedules and need to be uniquely identifiable as the data that is used will differ slightly.

Configuration

test-config:
  tasks:
    task-one:
      schedule: 0 0 0 ? * Sat *
      enabled: true
      to-print: Task 1
    task-two:
      schedule: 0 0 0 ? * Mon-Fri *
      enabled: false
      to-print: Task 2

Spring Bean

@Configuration
public class ExampleTaskConfiguration {

    private final TaskConfiguration taskConfiguration;
    private final Clock clock;

    public ExampleTaskConfiguration(TaskConfiguration taskConfiguration,
                                    Clock clock) {
        this.taskConfiguration = taskConfiguration;
        this.clock = clock;
    }

    @EventListener(ContextRefreshedEvent.class)
    public void setupRecurringTasks(ContextRefreshedEvent event) {
        SchedulerClient schedulerClient = event.getApplicationContext().getBean(SchedulerClient.class);

        taskConfiguration.getTasks().forEach((key, configuration) -> {
            createTaskInstance(schedulerClient, recurringTask(), key, configuration);
        });
    }

    @Bean
    public Task<DataWithSchedule> recurringTask() {
        return Tasks.recurringWithPersistentSchedule("PrintDataTask", DataWithSchedule.class)
                .execute((taskInstance, executionContext) ->  System.out.printf("Data %s", taskInstance.getData().getData()));
    }

    private void createTaskInstance(SchedulerClient schedulerClient, Task<DataWithSchedule> task, String key, ScheduledTaskConfiguration config) {
        String cronExpression = config.getSchedule();
        if (!config.isEnabled() || StringUtils.isBlank(config.getSchedule())) {
            cronExpression = "-"; // Disabled schedule expression
        }
        CronSchedule cron = new CronSchedule(cronExpression, clock.getZone(), CronStyle.QUARTZ);

        SchedulableInstance<DataWithSchedule> taskInstance = task.schedulableInstance(key, new DataWithSchedule(cron, config.getToPrint()));
        try {
            schedulerClient.reschedule(taskInstance);
            logger.info("Updated recurring task for {} ({})", key, cronExpression);
        } catch (TaskInstanceNotFoundException ex) {
            schedulerClient.schedule(taskInstance);
            logger.info("Registered recurring task for {} ({})", key, cronExpression);
        }
    }
}

Addendum

There is a very good chance that I am just missing something important with how to actually write tasks in this manner. I'm happy to accept corrections.

A current workaround I intend to use is to just register the cron expressions from the configuration and schedule tasks as normal, then check if the task should do anything within the execution of the task rather than having the scheduler decide. While this works, I do wonder if there is a benefit to just not having these executions happen in the first place with the scheduler handling disabled schedules.