Merge "Fixes statsd returning too much data at once." into pi-dev

cmds/statsd/benchmark/metric_util.cpp

@@ -366,7 +366,7 @@ sp<StatsLogProcessor> CreateStatsLogProcessor(const long timeBaseSec, const Stat
     sp<AlarmMonitor> periodicAlarmMonitor;
     sp<StatsLogProcessor> processor = new StatsLogProcessor(
         uidMap, anomalyAlarmMonitor, periodicAlarmMonitor, timeBaseSec * NS_PER_SEC,
-        [](const ConfigKey&){});
+        [](const ConfigKey&){return true;});
     processor->OnConfigUpdated(timeBaseSec * NS_PER_SEC, key, config);
     return processor;
 }

cmds/statsd/src/StatsLogProcessor.cpp

@@ -75,7 +75,7 @@ StatsLogProcessor::StatsLogProcessor(const sp<UidMap>& uidMap,
                                      const sp<AlarmMonitor>& anomalyAlarmMonitor,
                                      const sp<AlarmMonitor>& periodicAlarmMonitor,
                                      const int64_t timeBaseNs,
-                                     const std::function<void(const ConfigKey&)>& sendBroadcast)
+                                     const std::function<bool(const ConfigKey&)>& sendBroadcast)
     : mUidMap(uidMap),
       mAnomalyAlarmMonitor(anomalyAlarmMonitor),
       mPeriodicAlarmMonitor(periodicAlarmMonitor),
@@ -465,12 +465,21 @@ void StatsLogProcessor::flushIfNecessaryLocked(
     // We suspect that the byteSize() computation is expensive, so we set a rate limit.
     size_t totalBytes = metricsManager.byteSize();
     mLastByteSizeTimes[key] = timestampNs;
+    bool requestDump = false;
     if (totalBytes >
         StatsdStats::kMaxMetricsBytesPerConfig) {  // Too late. We need to start clearing data.
         metricsManager.dropData(timestampNs);
         StatsdStats::getInstance().noteDataDropped(key);
         VLOG("StatsD had to toss out metrics for %s", key.ToString().c_str());
-    } else if (totalBytes > StatsdStats::kBytesPerConfigTriggerGetData) {
+    } else if ((totalBytes > StatsdStats::kBytesPerConfigTriggerGetData) ||
+               (mOnDiskDataConfigs.find(key) != mOnDiskDataConfigs.end())) {
+        // Request to send a broadcast if:
+        // 1. in memory data > threshold   OR
+        // 2. config has old data report on disk.
+        requestDump = true;
+    }
+
+    if (requestDump) {
         // Send broadcast so that receivers can pull data.
         auto lastBroadcastTime = mLastBroadcastTimes.find(key);
         if (lastBroadcastTime != mLastBroadcastTimes.end()) {
@@ -479,10 +488,12 @@ void StatsLogProcessor::flushIfNecessaryLocked(
                 return;
             }
         }
-        mLastBroadcastTimes[key] = timestampNs;
+        if (mSendBroadcast(key)) {
-        VLOG("StatsD requesting broadcast for %s", key.ToString().c_str());
+            mOnDiskDataConfigs.erase(key);
-        mSendBroadcast(key);
+            VLOG("StatsD triggered data fetch for %s", key.ToString().c_str());
-        StatsdStats::getInstance().noteBroadcastSent(key);
+            mLastBroadcastTimes[key] = timestampNs;
+            StatsdStats::getInstance().noteBroadcastSent(key);
+        }
     }
 }
 
@@ -505,6 +516,8 @@ void StatsLogProcessor::WriteDataToDiskLocked(const ConfigKey& key,
         return;
     }
     proto.flush(fd.get());
+    // We were able to write the ConfigMetricsReport to disk, so we should trigger collection ASAP.
+    mOnDiskDataConfigs.insert(key);
 }
 
 void StatsLogProcessor::WriteDataToDiskLocked(const DumpReportReason dumpReportReason) {
@@ -533,6 +546,11 @@ int64_t StatsLogProcessor::getLastReportTimeNs(const ConfigKey& key) {
     }
 }
 
+void StatsLogProcessor::noteOnDiskData(const ConfigKey& key) {
+    std::lock_guard<std::mutex> lock(mMetricsMutex);
+    mOnDiskDataConfigs.insert(key);
+}
+
 }  // namespace statsd
 }  // namespace os
 }  // namespace android

cmds/statsd/src/StatsLogProcessor.h

@@ -48,7 +48,7 @@ public:
     StatsLogProcessor(const sp<UidMap>& uidMap, const sp<AlarmMonitor>& anomalyAlarmMonitor,
                       const sp<AlarmMonitor>& subscriberTriggerAlarmMonitor,
                       const int64_t timeBaseNs,
-                      const std::function<void(const ConfigKey&)>& sendBroadcast);
+                      const std::function<bool(const ConfigKey&)>& sendBroadcast);
     virtual ~StatsLogProcessor();
 
     void OnLogEvent(LogEvent* event, bool reconnectionStarts);
@@ -99,6 +99,9 @@ public:
 #endif
     }
 
+    // Add a specific config key to the possible configs to dump ASAP.
+    void noteOnDiskData(const ConfigKey& key);
+
 private:
     // For testing only.
     inline sp<AlarmMonitor> getAnomalyAlarmMonitor() const {
@@ -118,6 +121,9 @@ private:
     // Tracks when we last checked the bytes consumed for each config key.
     std::unordered_map<ConfigKey, long> mLastByteSizeTimes;
 
+    // Tracks which config keys has metric reports on disk
+    std::set<ConfigKey> mOnDiskDataConfigs;
+
     sp<UidMap> mUidMap;  // Reference to the UidMap to lookup app name and version for each uid.
 
     StatsPullerManager mStatsPullerManager;
@@ -159,7 +165,7 @@ private:
 
     // Function used to send a broadcast so that receiver for the config key can call getData
     // to retrieve the stored data.
-    std::function<void(const ConfigKey& key)> mSendBroadcast;
+    std::function<bool(const ConfigKey& key)> mSendBroadcast;
 
     const int64_t mTimeBaseNs;
 

cmds/statsd/src/StatsService.cpp

@@ -158,11 +158,14 @@ StatsService::StatsService(const sp<Looper>& handlerLooper)
         auto receiver = mConfigManager->GetConfigReceiver(key);
         if (sc == nullptr) {
             VLOG("Could not find StatsCompanionService");
+            return false;
         } else if (receiver == nullptr) {
             VLOG("Statscompanion could not find a broadcast receiver for %s",
                  key.ToString().c_str());
+            return false;
         } else {
             sc->sendDataBroadcast(receiver, mProcessor->getLastReportTimeNs(key));
+            return true;
         }
     }
     );
@@ -948,6 +951,11 @@ Status StatsService::setDataFetchOperation(int64_t key,
     IPCThreadState* ipc = IPCThreadState::self();
     ConfigKey configKey(ipc->getCallingUid(), key);
     mConfigManager->SetConfigReceiver(configKey, intentSender);
+    if (StorageManager::hasConfigMetricsReport(configKey)) {
+        VLOG("StatsService::setDataFetchOperation marking configKey %s to dump reports on disk",
+             configKey.ToString().c_str());
+        mProcessor->noteOnDiskData(configKey);
+    }
     return Status::ok();
 }
 

cmds/statsd/src/metrics/MetricsManager.cpp

@@ -62,7 +62,7 @@ MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config,
     : mConfigKey(key), mUidMap(uidMap),
       mTtlNs(config.has_ttl_in_seconds() ? config.ttl_in_seconds() * NS_PER_SEC : -1),
       mTtlEndNs(-1),
-      mLastReportTimeNs(timeBaseNs),
+      mLastReportTimeNs(currentTimeNs),
       mLastReportWallClockNs(getWallClockNs()) {
     // Init the ttl end timestamp.
     refreshTtl(timeBaseNs);

cmds/statsd/src/metrics/MetricsManager.h

@@ -79,7 +79,8 @@ public:
         }
     };
 
-    // Returns the elapsed realtime when this metric manager last reported metrics.
+    // Returns the elapsed realtime when this metric manager last reported metrics. If this config
+    // has not yet dumped any reports, this is the time the metricsmanager was initialized.
     inline int64_t getLastReportTimeNs() const {
         return mLastReportTimeNs;
     };

cmds/statsd/src/storage/StorageManager.cpp

@@ -160,6 +160,34 @@ void StorageManager::sendBroadcast(const char* path,
     }
 }
 
+bool StorageManager::hasConfigMetricsReport(const ConfigKey& key) {
+    unique_ptr<DIR, decltype(&closedir)> dir(opendir(STATS_DATA_DIR), closedir);
+    if (dir == NULL) {
+        VLOG("Path %s does not exist", STATS_DATA_DIR);
+        return false;
+    }
+
+    string suffix = StringPrintf("%d_%lld", key.GetUid(), (long long)key.GetId());
+
+    dirent* de;
+    while ((de = readdir(dir.get()))) {
+        char* name = de->d_name;
+        if (name[0] == '.') continue;
+
+        size_t nameLen = strlen(name);
+        size_t suffixLen = suffix.length();
+        if (suffixLen <= nameLen &&
+            strncmp(name + nameLen - suffixLen, suffix.c_str(), suffixLen) == 0) {
+            // Check again that the file name is parseable.
+            int64_t result[3];
+            parseFileName(name, result);
+            if (result[0] == -1) continue;
+            return true;
+        }
+    }
+    return false;
+}
+
 void StorageManager::appendConfigMetricsReport(const ConfigKey& key, ProtoOutputStream* proto) {
     unique_ptr<DIR, decltype(&closedir)> dir(opendir(STATS_DATA_DIR), closedir);
     if (dir == NULL) {

cmds/statsd/src/storage/StorageManager.h

@@ -62,6 +62,11 @@ public:
     static void sendBroadcast(const char* path,
                               const std::function<void(const ConfigKey&)>& sendBroadcast);
 
+    /**
+     * Returns true if there's at least one report on disk.
+     */
+    static bool hasConfigMetricsReport(const ConfigKey& key);
+
     /**
      * Appends ConfigMetricsReport found on disk to the specific proto and
      * delete it.

cmds/statsd/tests/StatsLogProcessor_test.cpp

@@ -62,7 +62,7 @@ TEST(StatsLogProcessorTest, TestRateLimitByteSize) {
     sp<AlarmMonitor> periodicAlarmMonitor;
     // Construct the processor with a dummy sendBroadcast function that does nothing.
     StatsLogProcessor p(m, anomalyAlarmMonitor, periodicAlarmMonitor, 0,
-        [](const ConfigKey& key) {});
+        [](const ConfigKey& key) {return true;});
 
     MockMetricsManager mockMetricsManager;
 
@@ -81,7 +81,7 @@ TEST(StatsLogProcessorTest, TestRateLimitBroadcast) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     int broadcastCount = 0;
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
 
     MockMetricsManager mockMetricsManager;
 
@@ -107,7 +107,7 @@ TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     int broadcastCount = 0;
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
 
     MockMetricsManager mockMetricsManager;
 
@@ -131,7 +131,7 @@ TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     int broadcastCount = 0;
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
     ConfigKey key(3, 4);
     StatsdConfig config;
     config.add_allowed_log_source("AID_ROOT");
@@ -156,7 +156,7 @@ TEST(StatsLogProcessorTest, TestReportIncludesSubConfig) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     int broadcastCount = 0;
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
     ConfigKey key(3, 4);
     StatsdConfig config;
     auto annotation = config.add_annotation();
@@ -185,7 +185,7 @@ TEST(StatsLogProcessorTest, TestOutOfOrderLogs) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     int broadcastCount = 0;
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; });
+                        [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
 
     LogEvent event1(0, 1 /*logd timestamp*/, 1001 /*elapsedRealtime*/);
     event1.init();

cmds/statsd/tests/UidMap_test.cpp

@@ -44,7 +44,7 @@ TEST(UidMapTest, TestIsolatedUID) {
     sp<AlarmMonitor> subscriberAlarmMonitor;
     // Construct the processor with a dummy sendBroadcast function that does nothing.
     StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
-        [](const ConfigKey& key) {});
+        [](const ConfigKey& key) {return true;});
     LogEvent addEvent(android::util::ISOLATED_UID_CHANGED, 1);
     addEvent.write(100);  // parent UID
     addEvent.write(101);  // isolated UID

cmds/statsd/tests/statsd_test_util.cpp

@@ -459,7 +459,7 @@ sp<StatsLogProcessor> CreateStatsLogProcessor(const int64_t timeBaseNs, const in
         new AlarmMonitor(1,  [](const sp<IStatsCompanionService>&, int64_t){},
                 [](const sp<IStatsCompanionService>&){});
     sp<StatsLogProcessor> processor = new StatsLogProcessor(
-        uidMap, anomalyAlarmMonitor, periodicAlarmMonitor, timeBaseNs, [](const ConfigKey&){});
+        uidMap, anomalyAlarmMonitor, periodicAlarmMonitor, timeBaseNs, [](const ConfigKey&){return true;});
     processor->OnConfigUpdated(currentTimeNs, key, config);
     return processor;
 }