Fix/updater automated update max interval of 23 hours

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/backup/proto/ProtoBackupExport.kt

@@ -54,7 +54,7 @@ object ProtoBackupExport : ProtoBackupBase() {
     private val preferences = Preferences.userNodeForPackage(ProtoBackupExport::class.java)
 
     fun scheduleAutomatedBackupTask() {
-        HAScheduler.deschedule(backupSchedulerJobId)
+        HAScheduler.descheduleCron(backupSchedulerJobId)
 
         if (!serverConfig.automatedBackups) {
             return
@@ -79,7 +79,7 @@ object ProtoBackupExport : ProtoBackupBase() {
             task()
         }
 
-        HAScheduler.schedule(task, "$backupMinute $backupHour */${backupInterval.inWholeDays} * *", "backup")
+        HAScheduler.scheduleCron(task, "$backupMinute $backupHour */${backupInterval.inWholeDays} * *", "backup")
     }
 
     private fun createAutomatedBackup() {

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/update/Updater.kt

@@ -72,23 +72,16 @@ class Updater : IUpdater {
     private fun scheduleUpdateTask() {
         HAScheduler.deschedule(currentUpdateTaskId)
 
-        if (!serverConfig.automaticallyTriggerGlobalUpdate) {
+        val isAutoUpdateDisabled = serverConfig.globalUpdateInterval == 0.0
+        if (isAutoUpdateDisabled) {
             return
         }
 
-        val minInterval = 6.hours
-        val interval = serverConfig.globalUpdateInterval.hours
-        val updateInterval = interval.coerceAtLeast(minInterval)
-        val lastAutomatedUpdate = preferences.getLong(lastAutomatedUpdateKey, System.currentTimeMillis())
-
-        // trigger update in case the server wasn't running on the scheduled time
-        val wasPreviousUpdateTriggered =
-            (System.currentTimeMillis() - lastAutomatedUpdate) < updateInterval.inWholeMilliseconds
-        if (!wasPreviousUpdateTriggered) {
-            autoUpdateTask()
-        }
+        val updateInterval = serverConfig.globalUpdateInterval.hours.coerceAtLeast(6.hours).inWholeMilliseconds
+        val lastAutomatedUpdate = preferences.getLong(lastAutomatedUpdateKey, 0)
+        val initialDelay = updateInterval - (System.currentTimeMillis() - lastAutomatedUpdate) % updateInterval
 
-        HAScheduler.schedule(::autoUpdateTask, "0 */${updateInterval.inWholeHours} * * *", "global-update")
+        HAScheduler.schedule(::autoUpdateTask, updateInterval, initialDelay, "global-update")
     }
 
     private fun getOrCreateUpdateChannelFor(source: String): Channel<UpdateJob> {

server/src/main/kotlin/suwayomi/tachidesk/server/ServerConfig.kt

@@ -41,7 +41,6 @@ class ServerConfig(getConfig: () -> Config, moduleName: String = MODULE_NAME) :
     var excludeUnreadChapters: Boolean by overridableConfig
     var excludeNotStarted: Boolean by overridableConfig
     var excludeCompleted: Boolean by overridableConfig
-    var automaticallyTriggerGlobalUpdate: Boolean by overridableConfig
     var globalUpdateInterval: Double by overridableConfig
 
     // Authentication

server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt

@@ -76,7 +76,7 @@ object WebInterfaceManager {
     }
 
     private fun scheduleWebUIUpdateCheck() {
-        HAScheduler.deschedule(currentUpdateTaskId)
+        HAScheduler.descheduleCron(currentUpdateTaskId)
 
         val isAutoUpdateDisabled = !isAutoUpdateEnabled() || serverConfig.webUIFlavor == "Custom"
         if (isAutoUpdateDisabled) {
@@ -96,8 +96,7 @@ object WebInterfaceManager {
             task()
         }
 
-        HAScheduler.deschedule(currentUpdateTaskId)
-        currentUpdateTaskId = HAScheduler.schedule(task, "0 */${updateInterval.inWholeHours} * * *", "webUI-update-checker")
+        currentUpdateTaskId = HAScheduler.scheduleCron(task, "0 */${updateInterval.inWholeHours} * * *", "webUI-update-checker")
     }
 
     fun setupWebUI() {

server/src/main/kotlin/suwayomi/tachidesk/util/HAScheduler.kt

@@ -13,29 +13,59 @@ import java.time.ZonedDateTime
 import java.util.PriorityQueue
 import java.util.Timer
 import java.util.TimerTask
+import java.util.UUID
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.minutes
 import kotlin.time.Duration.Companion.seconds
 
 val cronParser = CronParser(CronDefinitionBuilder.instanceDefinitionFor(CRON4J))
 
-class HATask(val id: String, val cronExpr: String, val execute: () -> Unit, val name: String?) : Comparable<HATask> {
+abstract class BaseHATask(val id: String, val execute: () -> Unit, val name: String?) : Comparable<BaseHATask> {
+    abstract fun getLastExecutionTime(): Long
+
+    abstract fun getNextExecutionTime(): Long
+
+    abstract fun getTimeToNextExecution(): Long
+
+    override fun compareTo(other: BaseHATask): Int {
+        return getTimeToNextExecution().compareTo(other.getTimeToNextExecution())
+    }
+}
+
+class HACronTask(id: String, val cronExpr: String, execute: () -> Unit, name: String?) : BaseHATask(id, execute, name) {
     private val executionTime = ExecutionTime.forCron(cronParser.parse(cronExpr))
 
-    fun getLastExecutionTime(): Long {
+    override fun getLastExecutionTime(): Long {
         return executionTime.lastExecution(ZonedDateTime.now()).get().toEpochSecond().seconds.inWholeMilliseconds
     }
 
-    fun getNextExecutionTime(): Long {
+    override fun getNextExecutionTime(): Long {
         return executionTime.nextExecution(ZonedDateTime.now()).get().toEpochSecond().seconds.inWholeMilliseconds
     }
 
-    fun getTimeToNextExecution(): Long {
+    override fun getTimeToNextExecution(): Long {
         return executionTime.timeToNextExecution(ZonedDateTime.now()).get().toMillis()
     }
+}
 
-    override fun compareTo(other: HATask): Int {
-        return getTimeToNextExecution().compareTo(other.getTimeToNextExecution())
+class HATask(id: String, val interval: Long, execute: () -> Unit, val timerTask: TimerTask, name: String?) : BaseHATask(id, execute, name) {
+    private val firstExecutionTime = System.currentTimeMillis() + interval
+
+    private fun getElapsedTimeOfCurrentInterval(): Long {
+        val timeSinceFirstExecution = System.currentTimeMillis() - firstExecutionTime
+        return timeSinceFirstExecution % interval
+    }
+
+    override fun getLastExecutionTime(): Long {
+        return System.currentTimeMillis() - getElapsedTimeOfCurrentInterval()
+    }
+
+    override fun getNextExecutionTime(): Long {
+        return System.currentTimeMillis() + getTimeToNextExecution()
+    }
+
+    override fun getTimeToNextExecution(): Long {
+        return interval - getElapsedTimeOfCurrentInterval()
     }
 }
 
@@ -46,9 +76,11 @@ class HATask(val id: String, val cronExpr: String, val execute: () -> Unit, val
 object HAScheduler {
     private val logger = KotlinLogging.logger { }
 
-    private val scheduledTasks = PriorityQueue<HATask>()
+    private val scheduledTasks = PriorityQueue<BaseHATask>()
     private val scheduler = Scheduler()
 
+    private val timer = Timer()
+
     private val HIBERNATION_THRESHOLD = 10.seconds.inWholeMilliseconds
     private const val TASK_THRESHOLD = 0.1
 
@@ -57,7 +89,6 @@ object HAScheduler {
     }
 
     private fun scheduleHibernateCheckerTask(interval: Duration) {
-        val timer = Timer()
         timer.scheduleAtFixedRate(
             object : TimerTask() {
                 var lastExecutionTime = System.currentTimeMillis()
@@ -79,8 +110,14 @@ object HAScheduler {
                             val triggerTask = missedExecution && taskThresholdMet
                             if (triggerTask) {
                                 logger.debug { "Task \"${it.name ?: it.id}\" missed its execution, executing now..." }
-                                reschedule(it.id, it.cronExpr)
-                                it.execute()
+
+                                when (it) {
+                                    is HATask -> reschedule(it.id, it.interval)
+                                    is HACronTask -> {
+                                        rescheduleCron(it.id, it.cronExpr)
+                                        it.execute()
+                                    }
+                                }
                             }
 
                             // queue is ordered by next execution time, thus, loop can be exited early
@@ -96,7 +133,62 @@ object HAScheduler {
         )
     }
 
-    fun schedule(execute: () -> Unit, cronExpr: String, name: String?): String {
+    private fun createTimerTask(interval: Long, execute: () -> Unit): TimerTask {
+        return object : TimerTask() {
+            var lastExecutionTime: Long = 0
+
+            override fun run() {
+                // If a task scheduled via "Timer::scheduleAtFixedRate" is delayed for some reason, the Timer will
+                // trigger tasks in quick succession to "catch up" to the set interval.
+                //
+                // We want to prevent this, since we don't care about how many executions were missed and only want
+                // one execution to be triggered for these missed executions.
+                //
+                // The missed execution gets triggered by "HAScheduler::scheduleHibernateCheckerTask" and thus, we
+                // debounce this behaviour of "Timer::scheduleAtFixedRate".
+                val isCatchUpExecution = System.currentTimeMillis() - lastExecutionTime < interval - HIBERNATION_THRESHOLD
+                if (isCatchUpExecution) {
+                    return
+                }
+
+                lastExecutionTime = System.currentTimeMillis()
+                execute()
+            }
+        }
+    }
+
+    fun schedule(execute: () -> Unit, interval: Long, delay: Long, name: String?): String {
+        val taskId = UUID.randomUUID().toString()
+        val task = createTimerTask(interval, execute)
+
+        scheduledTasks.add(HATask(taskId, interval, execute, task, name))
+        timer.scheduleAtFixedRate(task, delay, interval)
+
+        return taskId
+    }
+
+    fun deschedule(taskId: String): HATask? {
+        val task = (scheduledTasks.find { it.id == taskId } ?: return null) as HATask
+        task.timerTask.cancel()
+        scheduledTasks.remove(task)
+
+        return task
+    }
+
+    fun reschedule(taskId: String, interval: Long) {
+        val task = deschedule(taskId) ?: return
+
+        val timerTask = createTimerTask(interval, task.execute)
+
+        val timeToNextExecution = task.getTimeToNextExecution()
+        val intervalDifference = interval - task.interval
+        val remainingTimeTillNextExecution = (timeToNextExecution + intervalDifference).coerceAtLeast(0)
+
+        scheduledTasks.add(HATask(taskId, interval, task.execute, timerTask, task.name))
+        timer.scheduleAtFixedRate(timerTask, remainingTimeTillNextExecution, interval)
+    }
+
+    fun scheduleCron(execute: () -> Unit, cronExpr: String, name: String?): String {
         if (!scheduler.isStarted) {
             scheduler.start()
         }
@@ -110,21 +202,21 @@ object HAScheduler {
             }
         )
 
-        scheduledTasks.add(HATask(taskId, cronExpr, execute, name))
+        scheduledTasks.add(HACronTask(taskId, cronExpr, execute, name))
 
         return taskId
     }
 
-    fun deschedule(taskId: String) {
+    fun descheduleCron(taskId: String) {
         scheduler.deschedule(taskId)
         scheduledTasks.removeIf { it.id == taskId }
     }
 
-    fun reschedule(taskId: String, cronExpr: String) {
+    fun rescheduleCron(taskId: String, cronExpr: String) {
         val task = scheduledTasks.find { it.id == taskId } ?: return
 
         scheduledTasks.remove(task)
-        scheduledTasks.add(HATask(taskId, cronExpr, task.execute, task.name))
+        scheduledTasks.add(HACronTask(taskId, cronExpr, task.execute, task.name))
 
         scheduler.reschedule(taskId, cronExpr)
     }

server/src/main/resources/server-reference.conf

@@ -26,8 +26,7 @@ server.maxParallelUpdateRequests = 10 # sets how many sources can be updated in
 server.excludeUnreadChapters = true
 server.excludeNotStarted = true
 server.excludeCompleted = true
-server.automaticallyTriggerGlobalUpdate = false
-server.globalUpdateInterval = 12 # time in hours (doesn't have to be full hours e.g. 12.5) - range: 6 <= n < ∞ - default: 12 hours - interval in which the global update will be automatically triggered
+server.globalUpdateInterval = 12 # time in hours - 0 to disable it - (doesn't have to be full hours e.g. 12.5) - range: 6 <= n < ∞ - default: 12 hours - interval in which the global update will be automatically triggered
 
 # Authentication
 server.basicAuthEnabled = false

server/src/test/resources/server-reference.conf

@@ -16,7 +16,6 @@ server.maxParallelUpdateRequests = 10
 server.excludeUnreadChapters = true
 server.excludeNotStarted = true
 server.excludeCompleted = true
-server.automaticallyTriggerGlobalUpdate = false
 server.globalUpdateInterval = 12
 
 # misc