commit 48944901f7e6334724efadda6c6b27d9e88fc9e2
author David Chen <dwchen@google.com> Thu May 03 10:29:11 2018 -0700
committer Yao Chen <yaochen@google.com> Fri May 04 17:09:16 2018 -0700
tree 10a4c19530b68bd17727694622f3d712c74aeae4
parent 11969b49191cfa156ada8d5034d5d39c2b77dc69

Fixes statsd returning too much data at once.

We observe a single ConfigMetricsReportList can be greater than the
safe size for the binder transaction buffer since we only check the
size of the current metrics in progress, but we also return the
previous reports stored on disk.

This change will attempt to send another ConfigMetricsReportList
as soon as possible if there's already a report on disk.

Also fixes a bug when trying to trigger data fetch before the client
has registered the corresponding dataFetchOperation.

Bug: 79201869
Test: Tested manually on marlin-eng
Change-Id: I2d3677162804a27e7a7a95d482d80c46bd994a67

cmds/statsd/src/StatsLogProcessor.cpp

diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index ed07acc..4d27948 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -75,7 +75,7 @@
                                      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 @@
     // 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 @@
                 return;
             }
         }
-        mLastBroadcastTimes[key] = timestampNs;
-        VLOG("StatsD requesting broadcast for %s", key.ToString().c_str());
-        mSendBroadcast(key);
-        StatsdStats::getInstance().noteBroadcastSent(key);
+        if (mSendBroadcast(key)) {
+            mOnDiskDataConfigs.erase(key);
+            VLOG("StatsD triggered data fetch for %s", key.ToString().c_str());
+            mLastBroadcastTimes[key] = timestampNs;
+            StatsdStats::getInstance().noteBroadcastSent(key);
+        }
     }
 }
 
@@ -505,6 +516,8 @@
         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 @@
     }
 }
 
+void StatsLogProcessor::noteOnDiskData(const ConfigKey& key) {
+    std::lock_guard<std::mutex> lock(mMetricsMutex);
+    mOnDiskDataConfigs.insert(key);
+}
+
 }  // namespace statsd
 }  // namespace os
 }  // namespace android