From 36cfdfa9bdadca4ce237798e6484be299c959f43 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Mon, 7 Feb 2022 17:47:01 +0000 Subject: [PATCH] Fix ACTIVE/EXEMPT quota calculation. We were incorrectly saying that jobs in the ACTIVE + EXEMPTED buckets (where bucket window size = allowed time) didn't have any extra time until the quota was consumed even though they were still under the max execution limit. Changing to make sure we don't return the incorrect value. Also, rescheduling the timer cutoff when the lowest priority of tracked jobs changes. Bug: 218216817 Test: atest FrameworksMockingServicesTests:QuotaControllerTest Change-Id: I98fb3466ca65a102f973c4f588d47c1f1b26ad6e --- .../job/controllers/QuotaController.java | 17 +++++++++-- .../job/controllers/QuotaControllerTest.java | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java index c1728a3a8abea..19f253719607a 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java @@ -1099,7 +1099,7 @@ public final class QuotaController extends StateController { final long maxExecutionTimeRemainingMs = mMaxExecutionTimeMs - stats.executionTimeInMaxPeriodMs; - if (allowedTimeRemainingMs <= 0 || maxExecutionTimeRemainingMs <= 0) { + if (maxExecutionTimeRemainingMs <= 0) { return 0; } @@ -1110,6 +1110,10 @@ public final class QuotaController extends StateController { sessions, startMaxElapsed, maxExecutionTimeRemainingMs); } + if (allowedTimeRemainingMs <= 0) { + return 0; + } + // Need to check both max time and period time in case one is less than the other. // For example, max time remaining could be less than bucket time remaining, but sessions // contributing to the max time remaining could phase out enough that we'd want to use the @@ -2147,6 +2151,7 @@ public final class QuotaController extends StateController { Slog.v(TAG, "Starting to track " + jobStatus.toShortString()); } // Always maintain list of running jobs, even when quota is free. + final boolean priorityLowered = mLowestPriority > jobStatus.getEffectivePriority(); mLowestPriority = Math.min(mLowestPriority, jobStatus.getEffectivePriority()); if (mRunningBgJobs.add(jobStatus) && shouldTrackLocked()) { mBgJobCount++; @@ -2163,6 +2168,8 @@ public final class QuotaController extends StateController { invalidateAllExecutionStatsLocked(mPkg.userId, mPkg.packageName); } scheduleCutoff(); + } else if (mRegularJobTimer && priorityLowered) { + scheduleCutoff(); } } } @@ -2188,12 +2195,18 @@ public final class QuotaController extends StateController { emitSessionLocked(nowElapsed); cancelCutoff(); mLowestPriority = JobInfo.PRIORITY_MAX; - } else if (mLowestPriority == jobStatus.getEffectivePriority()) { + } else if (mRegularJobTimer + && mLowestPriority == jobStatus.getEffectivePriority()) { + // Lowest priority doesn't matter for EJ timers. + final int oldPriority = mLowestPriority; mLowestPriority = JobInfo.PRIORITY_MAX; for (int i = mRunningBgJobs.size() - 1; i >= 0; --i) { mLowestPriority = Math.min(mLowestPriority, mRunningBgJobs.valueAt(i).getEffectivePriority()); } + if (mLowestPriority != oldPriority) { + scheduleCutoff(); + } } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java index 9d6793ca4b7b4..40bd800a346a9 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java @@ -321,6 +321,8 @@ public class QuotaControllerTest { private int bucketIndexToUsageStatsBucket(int bucketIndex) { switch (bucketIndex) { + case EXEMPTED_INDEX: + return UsageStatsManager.STANDBY_BUCKET_EXEMPTED; case ACTIVE_INDEX: return UsageStatsManager.STANDBY_BUCKET_ACTIVE; case WORKING_INDEX: @@ -1482,6 +1484,34 @@ public class QuotaControllerTest { } } + /** + * Test getTimeUntilQuotaConsumedLocked when allowed time equals the bucket window size. + */ + @Test + public void testGetTimeUntilQuotaConsumedLocked_AllowedEqualsWindow() { + final long now = JobSchedulerService.sElapsedRealtimeClock.millis(); + mQuotaController.saveTimingSession(SOURCE_USER_ID, SOURCE_PACKAGE, + createTimingSession(now - (8 * HOUR_IN_MILLIS), 20 * MINUTE_IN_MILLIS, 5), false); + mQuotaController.saveTimingSession(SOURCE_USER_ID, SOURCE_PACKAGE, + createTimingSession(now - (10 * MINUTE_IN_MILLIS), 10 * MINUTE_IN_MILLIS, 5), + false); + + setDeviceConfigLong(QcConstants.KEY_ALLOWED_TIME_PER_PERIOD_EXEMPTED_MS, + 10 * MINUTE_IN_MILLIS); + setDeviceConfigLong(QcConstants.KEY_WINDOW_SIZE_EXEMPTED_MS, 10 * MINUTE_IN_MILLIS); + // window size = allowed time, so jobs can essentially run non-stop until they reach the + // max execution time. + setStandbyBucket(EXEMPTED_INDEX); + synchronized (mQuotaController.mLock) { + assertEquals(0, + mQuotaController.getRemainingExecutionTimeLocked( + SOURCE_USER_ID, SOURCE_PACKAGE)); + assertEquals(mQcConstants.MAX_EXECUTION_TIME_MS - 30 * MINUTE_IN_MILLIS, + mQuotaController.getTimeUntilQuotaConsumedLocked( + SOURCE_USER_ID, SOURCE_PACKAGE, PRIORITY_DEFAULT)); + } + } + /** * Test getTimeUntilQuotaConsumedLocked when the determination is based within the bucket * window.