Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix edge case in statsd alarms

Browse Source 
Right now, if an alarm fires exactly on time (within 1 second), or
exactly on a multiple of its period, statsd will reset the alarm for the
same time, which can result a loop of statsd setting an alarm, the alarm
firing, statsd setting an alarm at the same time, etc.

The fix is that if an alarm fires on time, we should move to the next
period.

Test: atest statsd_test (added to a test, which would fail without the
code change)
Bug: 159098080

Change-Id: I150538cb1d3d5a2b943df2a32d92f8f84bb96626
This commit is contained in:
Tej Singh
2020-06-25 22:38:03 -07:00
parent 254b6cbd3a
commit f5cd94b4d0
2 changed files with 29 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cmds/statsd/src/anomaly/AlarmTracker.cpp
View File

@@ -60,11 +60,11 @@ void AlarmTracker::addSubscription(const Subscription& subscription) {
}
int64_t AlarmTracker::findNextAlarmSec(int64_t currentTimeSec) {
if (currentTimeSec <= mAlarmSec) {
if (currentTimeSec < mAlarmSec) {
return mAlarmSec;
}
int64_t periodsForward =
((currentTimeSec - mAlarmSec) * MS_PER_SEC - 1) / mAlarmConfig.period_millis() + 1;
((currentTimeSec - mAlarmSec) * MS_PER_SEC) / mAlarmConfig.period_millis() + 1;
return mAlarmSec + periodsForward * mAlarmConfig.period_millis() / MS_PER_SEC;
}

30
cmds/statsd/tests/anomaly/AlarmTracker_test.cpp
View File

@@ -43,23 +43,47 @@ TEST(AlarmTrackerTest, TestTriggerTimestamp) {
alarm.set_offset_millis(15 * MS_PER_SEC);
alarm.set_period_millis(60 * 60 * MS_PER_SEC); // 1hr
int64_t startMillis = 100000000 * MS_PER_SEC;
int64_t nextAlarmTime = startMillis / MS_PER_SEC + 15;
AlarmTracker tracker(startMillis, startMillis, alarm, kConfigKey, subscriberAlarmMonitor);
EXPECT_EQ(tracker.mAlarmSec, (int64_t)(startMillis / MS_PER_SEC + 15));
EXPECT_EQ(tracker.mAlarmSec, nextAlarmTime);
uint64_t currentTimeSec = startMillis / MS_PER_SEC + 10;
std::unordered_set<sp<const InternalAlarm>, SpHash<InternalAlarm>> firedAlarmSet =
subscriberAlarmMonitor->popSoonerThan(static_cast<uint32_t>(currentTimeSec));
EXPECT_TRUE(firedAlarmSet.empty());
tracker.informAlarmsFired(currentTimeSec * NS_PER_SEC, firedAlarmSet);
EXPECT_EQ(tracker.mAlarmSec, (int64_t)(startMillis / MS_PER_SEC + 15));
EXPECT_EQ(tracker.mAlarmSec, nextAlarmTime);
EXPECT_EQ(tracker.getAlarmTimestampSec(), nextAlarmTime);
currentTimeSec = startMillis / MS_PER_SEC + 7000;
nextAlarmTime = startMillis / MS_PER_SEC + 15 + 2 * 60 * 60;
firedAlarmSet = subscriberAlarmMonitor->popSoonerThan(static_cast<uint32_t>(currentTimeSec));
ASSERT_EQ(firedAlarmSet.size(), 1u);
tracker.informAlarmsFired(currentTimeSec * NS_PER_SEC, firedAlarmSet);
EXPECT_TRUE(firedAlarmSet.empty());
EXPECT_EQ(tracker.mAlarmSec, (int64_t)(startMillis / MS_PER_SEC + 15 + 2 * 60 * 60));
EXPECT_EQ(tracker.mAlarmSec, nextAlarmTime);
EXPECT_EQ(tracker.getAlarmTimestampSec(), nextAlarmTime);
// Alarm fires exactly on time.
currentTimeSec = startMillis / MS_PER_SEC + 15 + 2 * 60 * 60;
nextAlarmTime = startMillis / MS_PER_SEC + 15 + 3 * 60 * 60;
firedAlarmSet = subscriberAlarmMonitor->popSoonerThan(static_cast<uint32_t>(currentTimeSec));
ASSERT_EQ(firedAlarmSet.size(), 1u);
tracker.informAlarmsFired(currentTimeSec * NS_PER_SEC, firedAlarmSet);
EXPECT_TRUE(firedAlarmSet.empty());
EXPECT_EQ(tracker.mAlarmSec, nextAlarmTime);
EXPECT_EQ(tracker.getAlarmTimestampSec(), nextAlarmTime);
// Alarm fires exactly 1 period late.
currentTimeSec = startMillis / MS_PER_SEC + 15 + 4 * 60 * 60;
nextAlarmTime = startMillis / MS_PER_SEC + 15 + 5 * 60 * 60;
firedAlarmSet = subscriberAlarmMonitor->popSoonerThan(static_cast<uint32_t>(currentTimeSec));
ASSERT_EQ(firedAlarmSet.size(), 1u);
tracker.informAlarmsFired(currentTimeSec * NS_PER_SEC, firedAlarmSet);
EXPECT_TRUE(firedAlarmSet.empty());
EXPECT_EQ(tracker.mAlarmSec, nextAlarmTime);
EXPECT_EQ(tracker.getAlarmTimestampSec(), nextAlarmTime);
}
} // namespace statsd