From 0fac5b17e6ec11461a35859740212daa13d2c5cd Mon Sep 17 00:00:00 2001 From: Yao Chen Date: Tue, 28 Nov 2017 16:07:02 -0800 Subject: [PATCH] Add no more than 20 timestamps for a config in StatsdStats. To avoid the array to grow unboundedly. Test: added unit tests. Change-Id: I3c4823b2a89bb69428d81b9fe348e7ea9aba24c7 --- cmds/statsd/src/guardrail/StatsdStats.cpp | 44 ++++++++++++++++++++---- cmds/statsd/src/guardrail/StatsdStats.h | 9 +++++ cmds/statsd/tests/guardrail/StatsdStats_test.cpp | 42 ++++++++++++++++++++++ 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp index 2ab1146208ee..2bd36126d340 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.cpp +++ b/cmds/statsd/src/guardrail/StatsdStats.cpp @@ -117,36 +117,60 @@ void StatsdStats::noteConfigRemoved(const ConfigKey& key) { } void StatsdStats::noteBroadcastSent(const ConfigKey& key) { + noteBroadcastSent(key, time(nullptr)); +} + +void StatsdStats::noteBroadcastSent(const ConfigKey& key, int32_t timeSec) { lock_guard lock(mLock); auto it = mConfigStats.find(key); if (it == mConfigStats.end()) { ALOGE("Config key %s not found!", key.ToString().c_str()); return; } - - it->second.add_broadcast_sent_time_sec(time(nullptr)); + if (it->second.broadcast_sent_time_sec_size() >= kMaxTimestampCount) { + auto timestampList = it->second.mutable_broadcast_sent_time_sec(); + // This is O(N) operation. It shouldn't happen often, and N is only 20. + timestampList->erase(timestampList->begin()); + } + it->second.add_broadcast_sent_time_sec(timeSec); } void StatsdStats::noteDataDropped(const ConfigKey& key) { + noteDataDropped(key, time(nullptr)); +} + +void StatsdStats::noteDataDropped(const ConfigKey& key, int32_t timeSec) { lock_guard lock(mLock); auto it = mConfigStats.find(key); if (it == mConfigStats.end()) { ALOGE("Config key %s not found!", key.ToString().c_str()); return; } - - it->second.add_data_drop_time_sec(time(nullptr)); + if (it->second.data_drop_time_sec_size() >= kMaxTimestampCount) { + auto timestampList = it->second.mutable_data_drop_time_sec(); + // This is O(N) operation. It shouldn't happen often, and N is only 20. + timestampList->erase(timestampList->begin()); + } + it->second.add_data_drop_time_sec(timeSec); } void StatsdStats::noteMetricsReportSent(const ConfigKey& key) { + noteMetricsReportSent(key, time(nullptr)); +} + +void StatsdStats::noteMetricsReportSent(const ConfigKey& key, int32_t timeSec) { lock_guard lock(mLock); auto it = mConfigStats.find(key); if (it == mConfigStats.end()) { ALOGE("Config key %s not found!", key.ToString().c_str()); return; } - - it->second.add_dump_report_time_sec(time(nullptr)); + if (it->second.dump_report_time_sec_size() >= kMaxTimestampCount) { + auto timestampList = it->second.mutable_dump_report_time_sec(); + // This is O(N) operation. It shouldn't happen often, and N is only 20. + timestampList->erase(timestampList->begin()); + } + it->second.add_dump_report_time_sec(timeSec); } void StatsdStats::noteConditionDimensionSize(const ConfigKey& key, const string& name, int size) { @@ -201,6 +225,14 @@ void StatsdStats::resetInternalLocked() { mMetricsStats.clear(); std::fill(mPushedAtomStats.begin(), mPushedAtomStats.end(), 0); mMatcherStats.clear(); + for (auto& config : mConfigStats) { + config.second.clear_broadcast_sent_time_sec(); + config.second.clear_data_drop_time_sec(); + config.second.clear_dump_report_time_sec(); + config.second.clear_matcher_stats(); + config.second.clear_condition_stats(); + config.second.clear_metric_stats(); + } } void StatsdStats::addSubStatsToConfig(const ConfigKey& key, diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h index 6fd9e4b6cd33..451144f80bbf 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.h +++ b/cmds/statsd/src/guardrail/StatsdStats.h @@ -43,6 +43,8 @@ public: const static int kMaxMetricCountPerConfig = 300; const static int kMaxMatcherCountPerConfig = 500; + const static int kMaxTimestampCount = 20; + /** * Report a new config has been received and report the static stats about the config. * @@ -162,11 +164,18 @@ private: void addSubStatsToConfig(const ConfigKey& key, StatsdStatsReport_ConfigStats& configStats); + void noteDataDropped(const ConfigKey& key, int32_t timeSec); + + void noteMetricsReportSent(const ConfigKey& key, int32_t timeSec); + + void noteBroadcastSent(const ConfigKey& key, int32_t timeSec); + FRIEND_TEST(StatsdStatsTest, TestValidConfigAdd); FRIEND_TEST(StatsdStatsTest, TestInvalidConfigAdd); FRIEND_TEST(StatsdStatsTest, TestConfigRemove); FRIEND_TEST(StatsdStatsTest, TestSubStats); FRIEND_TEST(StatsdStatsTest, TestAtomLog); + FRIEND_TEST(StatsdStatsTest, TestTimestampThreshold); }; } // namespace statsd diff --git a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp index b14b52cf595c..a8193dd92e8c 100644 --- a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp +++ b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp @@ -224,6 +224,48 @@ TEST(StatsdStatsTest, TestAtomLog) { EXPECT_TRUE(sensorAtomGood); } +TEST(StatsdStatsTest, TestTimestampThreshold) { + StatsdStats stats; + vector timestamps; + for (int i = 0; i < StatsdStats::kMaxTimestampCount; i++) { + timestamps.push_back(i); + } + ConfigKey key(0, "test"); + stats.noteConfigReceived(key, 2, 3, 4, 5, true); + + for (int i = 0; i < StatsdStats::kMaxTimestampCount; i++) { + stats.noteDataDropped(key, timestamps[i]); + stats.noteBroadcastSent(key, timestamps[i]); + stats.noteMetricsReportSent(key, timestamps[i]); + } + + int32_t newTimestamp = 10000; + + // now it should trigger removing oldest timestamp + stats.noteDataDropped(key, 10000); + stats.noteBroadcastSent(key, 10000); + stats.noteMetricsReportSent(key, 10000); + + EXPECT_TRUE(stats.mConfigStats.find(key) != stats.mConfigStats.end()); + const auto& configStats = stats.mConfigStats[key]; + + int maxCount = StatsdStats::kMaxTimestampCount; + EXPECT_EQ(maxCount, configStats.broadcast_sent_time_sec_size()); + EXPECT_EQ(maxCount, configStats.data_drop_time_sec_size()); + EXPECT_EQ(maxCount, configStats.dump_report_time_sec_size()); + + // the oldest timestamp is the second timestamp in history + EXPECT_EQ(1, configStats.broadcast_sent_time_sec(0)); + EXPECT_EQ(1, configStats.broadcast_sent_time_sec(0)); + EXPECT_EQ(1, configStats.broadcast_sent_time_sec(0)); + + // the last timestamp is the newest timestamp. + EXPECT_EQ(newTimestamp, + configStats.broadcast_sent_time_sec(StatsdStats::kMaxTimestampCount - 1)); + EXPECT_EQ(newTimestamp, configStats.data_drop_time_sec(StatsdStats::kMaxTimestampCount - 1)); + EXPECT_EQ(newTimestamp, configStats.dump_report_time_sec(StatsdStats::kMaxTimestampCount - 1)); +} + } // namespace statsd } // namespace os } // namespace android -- 2.11.0