From c02496c4f0df624ebf08521973314c43b945a3b8 Mon Sep 17 00:00:00 2001 From: schroda <50052685+schroda@users.noreply.github.com> Date: Sat, 22 Jul 2023 17:41:52 +0200 Subject: [PATCH] Fix/updater automated update max interval of 23 hours (#606) * Rename schedule functions * Introduce Base task for "HATask" * Support kotlin Timer repeated interval in HAScheduler It's not possible to schedule a task via cron expression to run every x hours in case the set hours are greater than 23. To be able to do this and still keep the functionality provided by the "HAScheduler" it has to also support repeated tasks scheduled via the default Timer * Support global update interval greater 23 hours * Use "globalUpdateInterval" to disable auto updates Gets rid of an unnecessary setting --- .../impl/backup/proto/ProtoBackupExport.kt | 4 +- .../tachidesk/manga/impl/update/Updater.kt | 19 +-- .../suwayomi/tachidesk/server/ServerConfig.kt | 1 - .../server/util/WebInterfaceManager.kt | 5 +- .../suwayomi/tachidesk/util/HAScheduler.kt | 122 +++++++++++++++--- .../src/main/resources/server-reference.conf | 3 +- .../src/test/resources/server-reference.conf | 1 - 7 files changed, 118 insertions(+), 37 deletions(-) diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/backup/proto/ProtoBackupExport.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/backup/proto/ProtoBackupExport.kt index 629210d9..474a72ee 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/backup/proto/ProtoBackupExport.kt +++ b/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) val areAutomatedBackupsDisabled = serverConfig.backupInterval == 0 if (areAutomatedBackupsDisabled) { @@ -80,7 +80,7 @@ object ProtoBackupExport : ProtoBackupBase() { task() } - HAScheduler.schedule(task, "$backupMinute $backupHour */${backupInterval.inWholeDays} * *", "backup") + HAScheduler.scheduleCron(task, "$backupMinute $backupHour */${backupInterval.inWholeDays} * *", "backup") } private fun createAutomatedBackup() { diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/update/Updater.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/update/Updater.kt index 820e23de..ec5d1ee9 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/update/Updater.kt +++ b/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()) + val updateInterval = serverConfig.globalUpdateInterval.hours.coerceAtLeast(6.hours).inWholeMilliseconds + val lastAutomatedUpdate = preferences.getLong(lastAutomatedUpdateKey, 0) + val initialDelay = updateInterval - (System.currentTimeMillis() - lastAutomatedUpdate) % updateInterval - // trigger update in case the server wasn't running on the scheduled time - val wasPreviousUpdateTriggered = - (System.currentTimeMillis() - lastAutomatedUpdate) < updateInterval.inWholeMilliseconds - if (!wasPreviousUpdateTriggered) { - autoUpdateTask() - } - - HAScheduler.schedule(::autoUpdateTask, "0 */${updateInterval.inWholeHours} * * *", "global-update") + HAScheduler.schedule(::autoUpdateTask, updateInterval, initialDelay, "global-update") } private fun getOrCreateUpdateChannelFor(source: String): Channel { diff --git a/server/src/main/kotlin/suwayomi/tachidesk/server/ServerConfig.kt b/server/src/main/kotlin/suwayomi/tachidesk/server/ServerConfig.kt index 0c98660c..737b0a4a 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/server/ServerConfig.kt +++ b/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 diff --git a/server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt b/server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt index cbef90a5..a5dd28ed 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt +++ b/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() { diff --git a/server/src/main/kotlin/suwayomi/tachidesk/util/HAScheduler.kt b/server/src/main/kotlin/suwayomi/tachidesk/util/HAScheduler.kt index fc78c691..4736e1d0 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/util/HAScheduler.kt +++ b/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 { +abstract class BaseHATask(val id: String, val execute: () -> Unit, val name: String?) : Comparable { + 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() + private val scheduledTasks = PriorityQueue() 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) } diff --git a/server/src/main/resources/server-reference.conf b/server/src/main/resources/server-reference.conf index a6fa2b0a..b790bac7 100644 --- a/server/src/main/resources/server-reference.conf +++ b/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 diff --git a/server/src/test/resources/server-reference.conf b/server/src/test/resources/server-reference.conf index e013582d..0e8040e8 100644 --- a/server/src/test/resources/server-reference.conf +++ b/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