From 6aabd50f5c893a4c0c0f9884086390121010eb55 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 13 Apr 2022 21:18:46 +0000 Subject: [PATCH] Fix rescheduling of jobs with large periods. We were accidentally skipping execution windows of jobs with large periods if the job was deferred by 30+ minutes, even if the next window would have been days later. For example, if a periodic job with a period of 7 days and flex of 1 day was scheduled at t0, its original window would be [t6, t7]. If it ended up running at t7+35minutes, JS would reschedule the next iteration for [t20, t21], even though it would be safe to schedule it for [t13, t14]. This change fixes the rescheduling so we don't skip the next window unless we are actually close to the start of the next window. Bug: 229135653 Test: atest FrameworksMockingServicesTests:JobSchedulerServiceTest Change-Id: I068628e96529b834c5b68e9e25fa130f40f0eef9 --- .../server/job/JobSchedulerService.java | 9 +- .../server/job/JobSchedulerServiceTest.java | 95 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java index aad8f9da62261..9e131339595fd 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java +++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java @@ -1895,11 +1895,14 @@ public class JobSchedulerService extends com.android.server.SystemService // The job ran past its expected run window. Have it count towards the current window // and schedule a new job for the next window. if (DEBUG) { - Slog.i(TAG, "Periodic job ran after its intended window."); + Slog.i(TAG, "Periodic job ran after its intended window by " + diffMs + " ms"); } long numSkippedWindows = (diffMs / period) + 1; // +1 to include original window - if (period != flex && diffMs > Math.min(PERIODIC_JOB_WINDOW_BUFFER, - (period - flex) / 2)) { + // Determine how far into a single period the job ran, and determine if it's too close + // to the start of the next period. If the difference between the start of the execution + // window and the previous execution time inside of the period is less than the + // threshold, then we say that the job ran too close to the next period. + if (period != flex && (period - flex - (diffMs % period)) <= flex / 6) { if (DEBUG) { Slog.d(TAG, "Custom flex job ran too close to next window."); } diff --git a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java index 6a27ecc7f0946..1753fc7a61e43 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java @@ -737,6 +737,101 @@ public class JobSchedulerServiceTest { assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); } + @Test + public void testGetRescheduleJobForPeriodic_outsideWindow_flex_failedJob_longPeriod() { + JobStatus job = createJobStatus( + "testGetRescheduleJobForPeriodic_outsideWindow_flex_failedJob_longPeriod", + createJobInfo().setPeriodic(7 * DAY_IN_MILLIS, 9 * HOUR_IN_MILLIS)); + JobStatus failedJob = mService.getRescheduleJobForFailureLocked(job); + // First window starts 6.625 days from now. + advanceElapsedClock(6 * DAY_IN_MILLIS + 15 * HOUR_IN_MILLIS); + long now = sElapsedRealtimeClock.millis(); + long nextWindowStartTime = now + 7 * DAY_IN_MILLIS; + long nextWindowEndTime = nextWindowStartTime + 9 * HOUR_IN_MILLIS; + + advanceElapsedClock(6 * HOUR_IN_MILLIS + MINUTE_IN_MILLIS); + // Say the job ran at the very end of its previous window. The intended JSS behavior is to + // have consistent windows, so the new window should start as soon as the previous window + // ended and end PERIOD time after the previous window ended. + JobStatus rescheduledJob = mService.getRescheduleJobForPeriodic(failedJob); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + advanceElapsedClock(DAY_IN_MILLIS); + // Say the job ran a day late. Since the period is massive compared to the flex, JSS should + // put the rescheduled job in the original window. + rescheduledJob = mService.getRescheduleJobForPeriodic(failedJob); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // 1 day before the start of the next window. Given the large period, respect the original + // next window. + advanceElapsedClock(nextWindowStartTime - sElapsedRealtimeClock.millis() - DAY_IN_MILLIS); + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // 1 hour before the start of the next window. It's too close to the next window, so the + // returned job should be for the window after. + long oneHourBeforeNextWindow = + nextWindowStartTime - sElapsedRealtimeClock.millis() - HOUR_IN_MILLIS; + long fiveMinsBeforeNextWindow = + nextWindowStartTime - sElapsedRealtimeClock.millis() - 5 * MINUTE_IN_MILLIS; + advanceElapsedClock(oneHourBeforeNextWindow); + nextWindowStartTime += 7 * DAY_IN_MILLIS; + nextWindowEndTime += 7 * DAY_IN_MILLIS; + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // 5 minutes before the start of the next window. It's too close to the next window, so the + // returned job should be for the window after. + advanceElapsedClock(fiveMinsBeforeNextWindow); + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + advanceElapsedClock(14 * DAY_IN_MILLIS); + // Say that the job ran at this point, probably because the phone was off the entire time. + // The next window should be consistent (start and end at the time it would have had the job + // run normally in previous windows). + nextWindowStartTime += 14 * DAY_IN_MILLIS; + nextWindowEndTime += 14 * DAY_IN_MILLIS; + + rescheduledJob = mService.getRescheduleJobForPeriodic(failedJob); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // Test original job again but with a huge delay from the original execution window + + // 1 day before the start of the next window. Given the large period, respect the original + // next window. + advanceElapsedClock(nextWindowStartTime - sElapsedRealtimeClock.millis() - DAY_IN_MILLIS); + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // 1 hour before the start of the next window. It's too close to the next window, so the + // returned job should be for the window after. + oneHourBeforeNextWindow = + nextWindowStartTime - sElapsedRealtimeClock.millis() - HOUR_IN_MILLIS; + fiveMinsBeforeNextWindow = + nextWindowStartTime - sElapsedRealtimeClock.millis() - 5 * MINUTE_IN_MILLIS; + advanceElapsedClock(oneHourBeforeNextWindow); + nextWindowStartTime += 7 * DAY_IN_MILLIS; + nextWindowEndTime += 7 * DAY_IN_MILLIS; + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + + // 5 minutes before the start of the next window. It's too close to the next window, so the + // returned job should be for the window after. + advanceElapsedClock(fiveMinsBeforeNextWindow); + rescheduledJob = mService.getRescheduleJobForPeriodic(job); + assertEquals(nextWindowStartTime, rescheduledJob.getEarliestRunTime()); + assertEquals(nextWindowEndTime, rescheduledJob.getLatestRunTimeElapsed()); + } + /** Tests that rare job batching works as expected. */ @Test public void testRareJobBatching() {