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
This commit is contained in:
Kweku Adams
2022-04-13 21:18:46 +00:00
parent 1408bbe923
commit 6aabd50f5c
2 changed files with 101 additions and 3 deletions

View File

@@ -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.");
}

View File

@@ -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() {