javascheduled-tasks

Java scheduler initial delay isn't correctly being calculated


I am trying to make it so that once a week at 8 AM, the program sends a message. It does send it at the correct day, but there is an issue with the time element. If it's 7:58 AM on the day, I want it to send it will wait till 8 AM to send it, but if I have to restart the program at all after 8 AM it sends it again with an initial delay of 0 or a negative number if it's past the day it needs to be sent. Changing the negative number to 0 will cause the message to be sent, which keeps the bug, being it would past the send time.

public class Reminder extends ListenerAdapter{
    @Override
    public void onReady(ReadyEvent event){
        final ZoneId zone = ZoneId.systemDefault();
        final ZoneId realzone = ZoneId.of(zone.getId());
        final ZonedDateTime zdt = ZonedDateTime.now(realzone);
        String day = String.valueOf(zdt.getDayOfWeek());
        String targetday = String.valueOf(DayOfWeek.WEDNESDAY);
        ZonedDateTime targettime = zdt.withHour(8).withMinute(0);
        ZonedDateTime currenttime = ZonedDateTime.now();
        long InitialDelay = currenttime.until(targettime, ChronoUnit.MILLIS);
        final Set<ZonedDateTime> targetTimes = Set.of(targettime);

        JDA bot = event.getJDA(); // <- JDA ...
        final long realChannelID = fakeID; //real
        final int period = 1000 * 60 * 60 * 24 * 7;
        TextChannel textChannel = event.getJDA().getTextChannelById(realChannelID);

        System.out.println(InitialDelay);

        ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);

        Runnable WeeklyMessageTask = () ->  {
                System.out.println("starting task");

                // for (ZonedDateTime time : targetTimes) {
                if (day.equals(targetday) && InitialDelay >= 0) {
                    System.out.println("Sending weekly message...");
                    switch (randomint()) {
                        case 1 -> textChannel2.sendMessage("Tiamat is Titillated...  Who is on board for putting an end to that?").queue();
                        case 2 -> textChannel2.sendMessage("It is Wednesday and the War rages... Who will join the fight?").queue();
                        case 3 -> textChannel2.sendMessage("Head count for tommorrow everyone?").queue();
                    }
                }
                if (!day.equals(targetday) && InitialDelay >= 0)
                System.out.println("NOT Sending weekly message...");
                System.out.println("ending task");
            };
        scheduler.scheduleAtFixedRate(WeeklyMessageTask, InitialDelay, period, TimeUnit.MILLISECONDS);
    }

    public int randomint(){
        Random rand = new Random();
        int max=3,min=1;
        int randomNum = rand.nextInt(max - min + 1) + min;
        return randomNum;
    }
}

I have tried changing the if statement to check if the value is negative to prevent extra messages from being sent which did not resolve the issue. I have also tried different time units to see if that is the issue (7 days, to period TimeUnit.Milliseconds). I've been able to replicate this issue outside of JDA's API, so I know it's not an issue with the API.

How can I better calculate the InitialDelay, to avoid this issue with the scheduler.scheduleAtFixedRate()?


Solution

  • If invoked at 11 AM, your code currently submits a task with a negative delay. This means the scheduled action executes immediately (at 11 AM), and will execute again one week later (at 11 AM ...).

    To reliable notify at 8 AM, you should ensure your delay is never negative, by delaying until next week if this week's notification time has already passed.

    I'd do this as follows:

        public void scheduleNotifications() {
            Thread.ofVirtual().start(() -> {
                for (;;) {
                    sleepUntil(nextNotification());
                    System.out.println("Hi!");
                }
            });
        }
    
        LocalDateTime nextNotification() {
            var next = LocalDate.now()
                    .with(ChronoField.DAY_OF_WEEK, 3) // this week's Wednessday
                    .atTime(8, 0);                    // at 8 AM
            if (next.isBefore(LocalDateTime.now())) { // if in the past
                next = next.plusWeeks(1);             // delay until next week
            }
            return next;
        }
        
        void sleepUntil(LocalDateTime time) {
            try {
                Thread.sleep(Duration.between(LocalDateTime.now(), time));
            } catch (InterruptedException e) {
                throw new RuntimeException(e);
            }       
        }
    

    The above solution repeatedly calculates the delay to account for the varying length of a week (if daylight time ends or starts, a week may be an hour longer or shorter than normal). That is, if we care about a consistent notification time in the face of daylight saving time, we can not use ScheduledThreadPoolExecutor.scheduleAtFixedRate and must manage the repetion ourselves. Fortunately, virtual threads make this easy.

    If you can't use virtual threads yet, you can use ScheduledExecutor.schedule instead:

    import java.time.Duration;
    import java.time.LocalDate;
    import java.time.LocalDateTime;
    import java.time.temporal.ChronoField;
    import java.util.concurrent.ScheduledThreadPoolExecutor;
    import java.util.concurrent.TimeUnit;
    import java.util.function.Supplier;
    
    public class Test {
    
        ScheduledThreadPoolExecutor exec = new ScheduledThreadPoolExecutor(1);
    
        public void scheduleNotifications() {
            scheduleRepeatedly(() -> System.out.println("Hi!"), () -> nextNotification());
        }
    
        void scheduleRepeatedly(Runnable action, Supplier<LocalDateTime> timeSupplier) {
            var delay = Duration.between(LocalDateTime.now(), timeSupplier.get());
            exec.schedule(() -> {
                action.run();
                scheduleRepeatedly(action, timeSupplier);
            }, delay.toMillis(), TimeUnit.MILLISECONDS);
        }
    
        LocalDateTime nextNotification() {
            var next = LocalDate.now().with(ChronoField.DAY_OF_WEEK, 3).atTime(8, 0);
            if (next.isBefore(LocalDateTime.now())) {
                next = next.plusWeeks(1);
            }
            return next;
        }
    }