Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 48944901 authored by David Chen's avatar David Chen Committed by Yao Chen
Browse files

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
parent 11969b49

cmds/statsd/benchmark/metric_util.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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

+24 −6
Original line number Diff line number Diff line
@@ -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,12 +488,14 @@ void StatsLogProcessor::flushIfNecessaryLocked( 
                return;

            }

        }

        if (mSendBroadcast(key)) {

            mOnDiskDataConfigs.erase(key);

            VLOG("StatsD triggered data fetch for %s", key.ToString().c_str());

            mLastBroadcastTimes[key] = timestampNs;

        VLOG("StatsD requesting broadcast for %s", key.ToString().c_str());

        mSendBroadcast(key);

            StatsdStats::getInstance().noteBroadcastSent(key);

        }

    }

}



void StatsLogProcessor::WriteDataToDiskLocked(const ConfigKey& key,

                                              const int64_t timestampNs,
@@ -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

+8 −2
Original line number Diff line number Diff line
@@ -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

+8 −0
Original line number Diff line number Diff line
@@ -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

+1 −1
Original line number Diff line number Diff line
@@ -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);