From 37c405d79fb76c579606499c0dc58661496e93a5 Mon Sep 17 00:00:00 2001 From: Siim Sammul Date: Thu, 28 Apr 2022 10:22:26 +0100 Subject: Update the rate limiting parameters to be a bit more strict in limiting, but keep the buffer longer before reset. Bug: 225173288 Test: atest DropboxRateLimiterTest Change-Id: I6042e0bb8a2d59fab6258d776bf7687167c69662 --- .../java/com/android/server/am/DropboxRateLimiter.java | 11 ++++++++--- .../com/android/server/am/DropboxRateLimiterTest.java | 16 ++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/am/DropboxRateLimiter.java b/services/core/java/com/android/server/am/DropboxRateLimiter.java index 18fb6a480f29..08a6719a016a 100644 --- a/services/core/java/com/android/server/am/DropboxRateLimiter.java +++ b/services/core/java/com/android/server/am/DropboxRateLimiter.java @@ -24,9 +24,14 @@ import com.android.internal.annotations.GuardedBy; /** Rate limiter for adding errors into dropbox. */ public class DropboxRateLimiter { - private static final long RATE_LIMIT_BUFFER_EXPIRY = 15 * DateUtils.SECOND_IN_MILLIS; - private static final long RATE_LIMIT_BUFFER_DURATION = 10 * DateUtils.SECOND_IN_MILLIS; - private static final int RATE_LIMIT_ALLOWED_ENTRIES = 5; + // After RATE_LIMIT_ALLOWED_ENTRIES have been collected (for a single breakdown of + // process/eventType) further entries will be rejected until RATE_LIMIT_BUFFER_DURATION has + // elapsed, after which the current count for this breakdown will be reset. + private static final long RATE_LIMIT_BUFFER_DURATION = 10 * DateUtils.MINUTE_IN_MILLIS; + // The time duration after which the rate limit buffer will be cleared. + private static final long RATE_LIMIT_BUFFER_EXPIRY = 3 * RATE_LIMIT_BUFFER_DURATION; + // The number of entries to keep per breakdown of process/eventType. + private static final int RATE_LIMIT_ALLOWED_ENTRIES = 6; @GuardedBy("mErrorClusterRecords") private final ArrayMap mErrorClusterRecords = new ArrayMap<>(); diff --git a/services/tests/servicestests/src/com/android/server/am/DropboxRateLimiterTest.java b/services/tests/servicestests/src/com/android/server/am/DropboxRateLimiterTest.java index d1390c68e130..e68a8a0f3af8 100644 --- a/services/tests/servicestests/src/com/android/server/am/DropboxRateLimiterTest.java +++ b/services/tests/servicestests/src/com/android/server/am/DropboxRateLimiterTest.java @@ -49,10 +49,11 @@ public class DropboxRateLimiterTest { assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); + assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); // Different processes and tags should not get rate limited either. assertFalse(mRateLimiter.shouldRateLimit("tag", "process2").shouldRateLimit()); assertFalse(mRateLimiter.shouldRateLimit("tag2", "process").shouldRateLimit()); - // The 6th entry of the same process should be rate limited. + // The 7th entry of the same process should be rate limited. assertTrue(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); } @@ -64,12 +65,13 @@ public class DropboxRateLimiterTest { assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); - // The 6th entry of the same process should be rate limited. + assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); + // The 7th entry of the same process should be rate limited. assertTrue(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); - // After 11 seconds there should be nothing left in the buffer and the same type of entry + // After 11 minutes there should be nothing left in the buffer and the same type of entry // should not get rate limited anymore. - mClock.setOffsetMillis(11000); + mClock.setOffsetMillis(11 * 60 * 1000); assertFalse(mRateLimiter.shouldRateLimit("tag", "process").shouldRateLimit()); } @@ -86,13 +88,15 @@ public class DropboxRateLimiterTest { mRateLimiter.shouldRateLimit("tag", "p").droppedCountSinceRateLimitActivated()); assertEquals(0, mRateLimiter.shouldRateLimit("tag", "p").droppedCountSinceRateLimitActivated()); + assertEquals(0, + mRateLimiter.shouldRateLimit("tag", "p").droppedCountSinceRateLimitActivated()); assertEquals(1, mRateLimiter.shouldRateLimit("tag", "p").droppedCountSinceRateLimitActivated()); assertEquals(2, mRateLimiter.shouldRateLimit("tag", "p").droppedCountSinceRateLimitActivated()); - // After 11 seconds the rate limiting buffer will be cleared and rate limiting will stop. - mClock.setOffsetMillis(11000); + // After 11 minutes the rate limiting buffer will be cleared and rate limiting will stop. + mClock.setOffsetMillis(11 * 60 * 1000); // The first call after rate limiting stops will still return the number of dropped events. assertEquals(2, -- cgit v1.2.3-59-g8ed1b