From b40021c5fa277131b22044610b0f763304d7ab51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C9=91rry=20Shiv=C9=91m?= Date: Thu, 20 Jun 2024 12:18:46 +0530 Subject: [PATCH] fix: Fix long running jobs getting executed multiple times (#14) --------- Signed-off-by: starry-shivam --- .../ktscheduler/scheduler/KtScheduler.kt | 8 ++++-- .../dev/starry/ktscheduler/KtSchedulerTest.kt | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/dev/starry/ktscheduler/scheduler/KtScheduler.kt b/src/main/kotlin/dev/starry/ktscheduler/scheduler/KtScheduler.kt index 4c2bf5c..c152c03 100644 --- a/src/main/kotlin/dev/starry/ktscheduler/scheduler/KtScheduler.kt +++ b/src/main/kotlin/dev/starry/ktscheduler/scheduler/KtScheduler.kt @@ -277,6 +277,11 @@ class KtScheduler( dueJobs.forEach { job -> logger.info("Processing due jobs...") + // Set the next run time for the job or remove it if it has no next run time. + // We do this before executing the job to ensure that the job is not executed again + // if its task is long-running and the next run time is already due. + setNextRunTimeOrRemoveJob(job, now) + // Execute the job. executor.execute( job = job, onSuccess = { handleJobCompletion(job, now) }, @@ -287,14 +292,13 @@ class KtScheduler( // Handles the completion of a job by updating the next run time or removing the job. private fun handleJobCompletion(job: Job, now: ZonedDateTime) { - setNextRunTimeOrRemoveJob(job, now) + logger.info("Job ${job.jobId} completed successfully") notifyJobComplete(job.jobId) } // Handles an error encountered while executing a job. private fun handleJobError(job: Job, now: ZonedDateTime, exception: Exception) { logger.severe("Error executing job ${job.jobId}: $exception") - setNextRunTimeOrRemoveJob(job, now) notifyJobError(job.jobId, exception) } diff --git a/src/test/kotlin/dev/starry/ktscheduler/KtSchedulerTest.kt b/src/test/kotlin/dev/starry/ktscheduler/KtSchedulerTest.kt index da5ff3a..9631584 100644 --- a/src/test/kotlin/dev/starry/ktscheduler/KtSchedulerTest.kt +++ b/src/test/kotlin/dev/starry/ktscheduler/KtSchedulerTest.kt @@ -26,6 +26,7 @@ import dev.starry.ktscheduler.triggers.OneTimeTrigger import junit.framework.TestCase.assertFalse import junit.framework.TestCase.assertTrue import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.test.runTest import org.junit.Test import java.time.ZonedDateTime @@ -289,6 +290,33 @@ class KtSchedulerTest { scheduler.shutdown() } + @Test + fun `scheduler should not execute job multiple times if it is still running`(): Unit = runTest { + val scheduler = KtScheduler() + + // Create a job that takes 2 seconds to execute + val job = Job( + jobId = "longRunningJob", + trigger = IntervalTrigger(intervalSeconds = 1), + nextRunTime = ZonedDateTime.now().plusSeconds(1), + callback = { delay(2000) } + ) + val eventListener = TestJobEventListener() + + scheduler.addJob(job) + scheduler.addEventListener(eventListener) + scheduler.start() + Thread.sleep(200) + // Job should not be completed yet + assertEquals(0, eventListener.completedJobs.size) + // Wait for enough time to ensure job has run + Thread.sleep(3000) + scheduler.shutdown() + // Assert that the job was only executed once + assertEquals(1, eventListener.completedJobs.size) + assertEquals("longRunningJob", eventListener.completedJobs[0]) + } + private fun createTestJob( jobId: String, runAt: ZonedDateTime = ZonedDateTime.now().plusSeconds(1),