OSDN Git Service

align metrics to 5min bundary
authorChenjie Yu <cjyu@google.com>
Fri, 15 Dec 2017 00:48:54 +0000 (16:48 -0800)
committerChenjie Yu <cjyu@google.com>
Sun, 17 Dec 2017 01:08:46 +0000 (17:08 -0800)
We use one alarm clock for all pulled atoms.
If metrics from different configs are not aligned,
the clock will be set to repeat at higher and higher
frequency, and consume a lot of battery.
Current implementation assumes a 5min minimum bucket
size. New metric start time is set to be aligned to
the start time of statsd in the next 5min.
So it will ignore events up to 5min.

align puller alarm to minute bundary

Test: unit test
Change-Id: I77ffa3c13de363c780b1000181b9a9b780dd0846

13 files changed:
cmds/statsd/src/StatsLogProcessor.cpp
cmds/statsd/src/StatsLogProcessor.h
cmds/statsd/src/external/StatsPullerManager.h
cmds/statsd/src/external/StatsPullerManagerImpl.cpp
cmds/statsd/src/external/StatsPullerManagerImpl.h
cmds/statsd/src/metrics/MetricsManager.cpp
cmds/statsd/src/metrics/MetricsManager.h
cmds/statsd/src/metrics/metrics_manager_util.cpp
cmds/statsd/src/metrics/metrics_manager_util.h
cmds/statsd/src/stats_util.h
cmds/statsd/tests/ConfigManager_test.cpp
cmds/statsd/tests/MetricsManager_test.cpp
cmds/statsd/tests/StatsLogProcessor_test.cpp

index a3e39b6..f6caca8 100644 (file)
@@ -24,6 +24,7 @@
 #include "android-base/stringprintf.h"
 #include "guardrail/StatsdStats.h"
 #include "metrics/CountMetricProducer.h"
+#include "external/StatsPullerManager.h"
 #include "stats_util.h"
 #include "storage/StorageManager.h"
 
@@ -63,10 +64,15 @@ const int FIELD_ID_UID_MAP = 2;
 StatsLogProcessor::StatsLogProcessor(const sp<UidMap>& uidMap,
                                      const sp<AnomalyMonitor>& anomalyMonitor,
                                      const std::function<void(const ConfigKey&)>& sendBroadcast)
-    : mUidMap(uidMap), mAnomalyMonitor(anomalyMonitor), mSendBroadcast(sendBroadcast) {
+    : mUidMap(uidMap),
+      mAnomalyMonitor(anomalyMonitor),
+      mSendBroadcast(sendBroadcast),
+      mTimeBaseSec(time(nullptr)) {
     // On each initialization of StatsLogProcessor, check stats-data directory to see if there is
     // any left over data to be read.
     StorageManager::sendBroadcast(STATS_DATA_DIR, mSendBroadcast);
+    StatsPullerManager statsPullerManager;
+    statsPullerManager.SetTimeBaseSec(mTimeBaseSec);
 }
 
 StatsLogProcessor::~StatsLogProcessor() {
@@ -108,7 +114,7 @@ void StatsLogProcessor::OnLogEvent(const LogEvent& msg) {
 
 void StatsLogProcessor::OnConfigUpdated(const ConfigKey& key, const StatsdConfig& config) {
     ALOGD("Updated configuration for key %s", key.ToString().c_str());
-    unique_ptr<MetricsManager> newMetricsManager = std::make_unique<MetricsManager>(key, config);
+    unique_ptr<MetricsManager> newMetricsManager = std::make_unique<MetricsManager>(key, config, mTimeBaseSec);
 
     auto it = mMetricsManagers.find(key);
     if (it == mMetricsManagers.end() && mMetricsManagers.size() > StatsdStats::kMaxConfigCount) {
index a4df23a..7ec4e4b 100644 (file)
@@ -76,6 +76,8 @@ private:
     // to retrieve the stored data.
     std::function<void(const ConfigKey& key)> mSendBroadcast;
 
+    const long mTimeBaseSec;
+
     FRIEND_TEST(StatsLogProcessorTest, TestRateLimitByteSize);
     FRIEND_TEST(StatsLogProcessorTest, TestRateLimitBroadcast);
     FRIEND_TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge);
index 2e803c9..00a1475 100644 (file)
@@ -22,33 +22,41 @@ namespace android {
 namespace os {
 namespace statsd {
 
-class StatsPullerManager{
+class StatsPullerManager {
  public:
-  virtual ~StatsPullerManager() {}
+    virtual ~StatsPullerManager() {}
 
-  virtual void RegisterReceiver(int tagId, wp<PullDataReceiver> receiver, long intervalMs) {
-    mPullerManager.RegisterReceiver(tagId, receiver, intervalMs);
-  };
+    virtual void RegisterReceiver(int tagId,
+                                  wp <PullDataReceiver> receiver,
+                                  long intervalMs) {
+        mPullerManager.RegisterReceiver(tagId, receiver, intervalMs);
+    };
 
-  virtual void UnRegisterReceiver(int tagId, wp<PullDataReceiver> receiver) {
-    mPullerManager.UnRegisterReceiver(tagId, receiver);
-  };
+    virtual void UnRegisterReceiver(int tagId, wp <PullDataReceiver> receiver) {
+        mPullerManager.UnRegisterReceiver(tagId, receiver);
+    };
 
-  // Verify if we know how to pull for this matcher
-  bool PullerForMatcherExists(int tagId) {
-    return mPullerManager.PullerForMatcherExists(tagId);
-  }
+    // Verify if we know how to pull for this matcher
+    bool PullerForMatcherExists(int tagId) {
+        return mPullerManager.PullerForMatcherExists(tagId);
+    }
 
-  void OnAlarmFired() {
-    mPullerManager.OnAlarmFired();
-  }
+    void OnAlarmFired() {
+        mPullerManager.OnAlarmFired();
+    }
 
-  virtual bool Pull(const int tagId, vector<std::shared_ptr<LogEvent>>* data) {
-    return mPullerManager.Pull(tagId, data);
-  }
+    virtual bool
+    Pull(const int tagId, vector<std::shared_ptr<LogEvent>>* data) {
+        return mPullerManager.Pull(tagId, data);
+    }
+
+    virtual void SetTimeBaseSec(const long timeBaseSec) {
+        mPullerManager.SetTimeBaseSec(timeBaseSec);
+    }
 
  private:
-  StatsPullerManagerImpl& mPullerManager = StatsPullerManagerImpl::GetInstance();
+    StatsPullerManagerImpl
+        & mPullerManager = StatsPullerManagerImpl::GetInstance();
 };
 
 }  // namespace statsd
index c4688a2..529ee2b 100644 (file)
@@ -44,7 +44,7 @@ namespace os {
 namespace statsd {
 
 StatsPullerManagerImpl::StatsPullerManagerImpl()
-    : mCurrentPullingInterval(LONG_MAX), mPullStartTimeMs(get_pull_start_time_ms()) {
+    : mCurrentPullingInterval(LONG_MAX) {
     shared_ptr<StatsPuller> statsCompanionServicePuller = make_shared<StatsCompanionServicePuller>();
     shared_ptr<StatsPuller> resourcePowerManagerPuller = make_shared<ResourcePowerManagerPuller>();
     shared_ptr<StatsPuller> cpuTimePerUidPuller = make_shared<CpuTimePerUidPuller>();
@@ -94,11 +94,6 @@ bool StatsPullerManagerImpl::PullerForMatcherExists(int tagId) const {
     return mPullers.find(tagId) != mPullers.end();
 }
 
-long StatsPullerManagerImpl::get_pull_start_time_ms() const {
-    // TODO: limit and align pull intervals to 10min boundaries if this turns out to be a problem
-    return time(nullptr) * 1000;
-}
-
 void StatsPullerManagerImpl::RegisterReceiver(int tagId, wp<PullDataReceiver> receiver,
                                               long intervalMs) {
     AutoMutex _l(mReceiversLock);
@@ -114,12 +109,17 @@ void StatsPullerManagerImpl::RegisterReceiver(int tagId, wp<PullDataReceiver> re
     receiverInfo.timeInfo.first = intervalMs;
     receivers.push_back(receiverInfo);
 
+    // Round it to the nearest minutes. This is the limit of alarm manager.
+    // In practice, we should limit it higher.
+    long roundedIntervalMs = intervalMs/1000/60 * 1000 * 60;
     // There is only one alarm for all pulled events. So only set it to the smallest denom.
-    if (intervalMs < mCurrentPullingInterval) {
+    if (roundedIntervalMs < mCurrentPullingInterval) {
         VLOG("Updating pulling interval %ld", intervalMs);
-        mCurrentPullingInterval = intervalMs;
+        mCurrentPullingInterval = roundedIntervalMs;
+        long currentTimeMs = time(nullptr) * 1000;
+        long nextAlarmTimeMs = currentTimeMs + mCurrentPullingInterval - (currentTimeMs - mTimeBaseSec * 1000) % mCurrentPullingInterval;
         if (mStatsCompanionService != nullptr) {
-            mStatsCompanionService->setPullingAlarms(mPullStartTimeMs, mCurrentPullingInterval);
+            mStatsCompanionService->setPullingAlarms(nextAlarmTimeMs, mCurrentPullingInterval);
         } else {
             VLOG("Failed to update pulling interval");
         }
@@ -146,7 +146,7 @@ void StatsPullerManagerImpl::UnRegisterReceiver(int tagId, wp<PullDataReceiver>
 void StatsPullerManagerImpl::OnAlarmFired() {
     AutoMutex _l(mReceiversLock);
 
-    uint64_t currentTimeMs = time(nullptr) * 1000;
+    uint64_t currentTimeMs = time(nullptr) /60 * 60 * 1000;
 
     vector<pair<int, vector<ReceiverInfo*>>> needToPull =
             vector<pair<int, vector<ReceiverInfo*>>>();
index 306cc32..7c59f66 100644 (file)
@@ -47,6 +47,8 @@ public:
 
     bool Pull(const int tagId, vector<std::shared_ptr<LogEvent>>* data);
 
+    void SetTimeBaseSec(long timeBaseSec) {mTimeBaseSec = timeBaseSec;};
+
 private:
     StatsPullerManagerImpl();
 
@@ -73,11 +75,8 @@ private:
 
     // for pulled metrics, it is important for the buckets to be aligned to multiple of smallest
     // bucket size. All pulled metrics start pulling based on this time, so that they can be
-    // correctly attributed to the correct buckets. Pulled data attach a timestamp which is the
-    // request time.
-    const long mPullStartTimeMs;
-
-    long get_pull_start_time_ms() const;
+    // correctly attributed to the correct buckets.
+    long mTimeBaseSec;
 
     LogEvent parse_pulled_data(String16 data);
 };
index b0f0135..b23d2e9 100644 (file)
@@ -44,9 +44,9 @@ namespace statsd {
 
 const int FIELD_ID_METRICS = 1;
 
-MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config) : mConfigKey(key) {
+MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec) : mConfigKey(key) {
     mConfigValid =
-            initStatsdConfig(key, config, mTagIds, mAllAtomMatchers, mAllConditionTrackers,
+            initStatsdConfig(key, config, timeBaseSec, mTagIds, mAllAtomMatchers, mAllConditionTrackers,
                              mAllMetricProducers, mAllAnomalyTrackers, mConditionToMetricMap,
                              mTrackerToMetricMap, mTrackerToConditionMap);
 
index 34ea667..738adc9 100644 (file)
@@ -34,7 +34,7 @@ namespace statsd {
 // A MetricsManager is responsible for managing metrics from one single config source.
 class MetricsManager {
 public:
-    MetricsManager(const ConfigKey& configKey, const StatsdConfig& config);
+    MetricsManager(const ConfigKey& configKey, const StatsdConfig& config, const long timeBaseSec);
 
     virtual ~MetricsManager();
 
index 943becb..23316ea 100644 (file)
@@ -188,7 +188,7 @@ bool initConditions(const ConfigKey& key, const StatsdConfig& config,
     return true;
 }
 
-bool initMetrics(const ConfigKey& key, const StatsdConfig& config,
+bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec,
                  const unordered_map<string, int>& logTrackerMap,
                  const unordered_map<string, int>& conditionTrackerMap,
                  const vector<sp<LogMatchingTracker>>& allAtomMatchers,
@@ -202,7 +202,13 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config,
                                 config.event_metric_size() + config.value_metric_size();
     allMetricProducers.reserve(allMetricsCount);
     StatsPullerManager statsPullerManager;
-    uint64_t startTimeNs = time(nullptr) * NS_PER_SEC;
+    // Align all buckets to same instant in MIN_BUCKET_SIZE_SEC, so that avoid alarm
+    // clock will not grow very aggressive. New metrics will be delayed up to
+    // MIN_BUCKET_SIZE_SEC before starting.
+    long currentTimeSec = time(nullptr);
+    uint64_t startTimeNs = (currentTimeSec + kMinBucketSizeSec -
+                            (currentTimeSec - timeBaseSec) % kMinBucketSizeSec) *
+                           NS_PER_SEC;
 
     // Build MetricProducers for each metric defined in config.
     // build CountMetricProducer
@@ -478,7 +484,7 @@ bool initAlerts(const StatsdConfig& config, const unordered_map<string, int>& me
     return true;
 }
 
-bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, set<int>& allTagIds,
+bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec, set<int>& allTagIds,
                       vector<sp<LogMatchingTracker>>& allAtomMatchers,
                       vector<sp<ConditionTracker>>& allConditionTrackers,
                       vector<sp<MetricProducer>>& allMetricProducers,
@@ -502,7 +508,7 @@ bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, set<int>
         return false;
     }
 
-    if (!initMetrics(key, config, logTrackerMap, conditionTrackerMap, allAtomMatchers,
+    if (!initMetrics(key, config, timeBaseSec, logTrackerMap, conditionTrackerMap, allAtomMatchers,
                      allConditionTrackers, allMetricProducers, conditionToMetricMap,
                      trackerToMetricMap, metricProducerMap)) {
         ALOGE("initMetricProducers failed");
index 8e9c8e3..3337332 100644 (file)
@@ -68,6 +68,7 @@ bool initConditions(const ConfigKey& key, const StatsdConfig& config,
 // input:
 // [key]: the config key that this config belongs to
 // [config]: the input config
+// [timeBaseSec]: start time base for all metrics
 // [logTrackerMap]: LogMatchingTracker name to index mapping from previous step.
 // [conditionTrackerMap]: condition name to index mapping
 // output:
@@ -76,7 +77,7 @@ bool initConditions(const ConfigKey& key, const StatsdConfig& config,
 //                          the list of MetricProducer index
 // [trackerToMetricMap]: contains the mapping from log tracker to MetricProducer index.
 bool initMetrics(
-        const ConfigKey& key, const StatsdConfig& config,
+        const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec,
         const std::unordered_map<std::string, int>& logTrackerMap,
         const std::unordered_map<std::string, int>& conditionTrackerMap,
         const std::unordered_map<int, std::vector<MetricConditionLink>>& eventConditionLinks,
@@ -88,7 +89,7 @@ bool initMetrics(
 
 // Initialize MetricsManager from StatsdConfig.
 // Parameters are the members of MetricsManager. See MetricsManager for declaration.
-bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, std::set<int>& allTagIds,
+bool initStatsdConfig(const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec, std::set<int>& allTagIds,
                       std::vector<sp<LogMatchingTracker>>& allAtomMatchers,
                       std::vector<sp<ConditionTracker>>& allConditionTrackers,
                       std::vector<sp<MetricProducer>>& allMetricProducers,
index 594561d..e6c33f3 100644 (file)
@@ -26,6 +26,9 @@ namespace statsd {
 
 #define DEFAULT_DIMENSION_KEY ""
 
+// Minimum bucket size in seconds
+const long kMinBucketSizeSec = 5 * 60;
+
 typedef std::string HashableDimensionKey;
 
 typedef std::map<std::string, HashableDimensionKey> ConditionKey;
index 696fddf..8c47b6b 100644 (file)
@@ -63,7 +63,7 @@ MATCHER_P(StatsdConfigEq, name, "") {
 
 TEST(ConfigManagerTest, TestFakeConfig) {
     auto metricsManager =
-            std::make_unique<MetricsManager>(ConfigKey(0, "test"), build_fake_config());
+            std::make_unique<MetricsManager>(ConfigKey(0, "test"), build_fake_config(), 1000);
     EXPECT_TRUE(metricsManager->isConfigValid());
 }
 
index c6a5310..3c8ccab 100644 (file)
@@ -42,6 +42,8 @@ using android::os::statsd::Predicate;
 
 const ConfigKey kConfigKey(0, "test");
 
+const long timeBaseSec = 1000;
+
 StatsdConfig buildGoodConfig() {
     StatsdConfig config;
     config.set_name("12345");
@@ -275,7 +277,7 @@ TEST(MetricsManagerTest, TestGoodConfig) {
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
 
-    EXPECT_TRUE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_TRUE(initStatsdConfig(kConfigKey, config, timeBaseSec,  allTagIds, allAtomMatchers,
                                  allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                  conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
     EXPECT_EQ(1u, allMetricProducers.size());
@@ -293,7 +295,7 @@ TEST(MetricsManagerTest, TestDimensionMetricsWithMultiTags) {
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
 
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
@@ -309,7 +311,7 @@ TEST(MetricsManagerTest, TestCircleLogMatcherDependency) {
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
 
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
@@ -324,7 +326,7 @@ TEST(MetricsManagerTest, TestMissingMatchers) {
     unordered_map<int, std::vector<int>> conditionToMetricMap;
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
@@ -339,7 +341,7 @@ TEST(MetricsManagerTest, TestMissingPredicate) {
     unordered_map<int, std::vector<int>> conditionToMetricMap;
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
@@ -355,7 +357,7 @@ TEST(MetricsManagerTest, TestCirclePredicateDependency) {
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
 
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
@@ -371,7 +373,7 @@ TEST(MetricsManagerTest, testAlertWithUnknownMetric) {
     unordered_map<int, std::vector<int>> trackerToMetricMap;
     unordered_map<int, std::vector<int>> trackerToConditionMap;
 
-    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, allTagIds, allAtomMatchers,
+    EXPECT_FALSE(initStatsdConfig(kConfigKey, config, timeBaseSec, allTagIds, allAtomMatchers,
                                   allConditionTrackers, allMetricProducers, allAnomalyTrackers,
                                   conditionToMetricMap, trackerToMetricMap, trackerToConditionMap));
 }
index aff06ba..a5c8875 100644 (file)
@@ -41,7 +41,7 @@ using android::util::ProtoOutputStream;
  */
 class MockMetricsManager : public MetricsManager {
 public:
-    MockMetricsManager() : MetricsManager(ConfigKey(1, "key"), StatsdConfig()) {
+    MockMetricsManager() : MetricsManager(ConfigKey(1, "key"), StatsdConfig(), 1000) {
     }
 
     MOCK_METHOD0(byteSize, size_t());