From 52b478b56a156f9c75329b7d25e6bd4583464400 Mon Sep 17 00:00:00 2001 From: Yao Chen Date: Tue, 27 Mar 2018 10:59:45 -0700 Subject: [PATCH] Update Guardrail. + Config count is 10 per uid + Update the limit for metrics, matchers, conditions, etc. Test: statsd_test Bug: 73122377 Change-Id: I3e1adfe318d1354a7c9d1bf484855661aa3a1fc8 --- cmds/statsd/src/StatsLogProcessor.cpp | 6 -- cmds/statsd/src/config/ConfigManager.cpp | 95 +++++++++++++++--------- cmds/statsd/src/config/ConfigManager.h | 7 +- cmds/statsd/src/guardrail/StatsdStats.h | 12 +-- 4 files changed, 66 insertions(+), 54 deletions(-) diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp index a35570bd7bba7..a458c07394a7d 100644 --- a/cmds/statsd/src/StatsLogProcessor.cpp +++ b/cmds/statsd/src/StatsLogProcessor.cpp @@ -199,11 +199,6 @@ void StatsLogProcessor::OnConfigUpdated(const ConfigKey& key, const StatsdConfig sp newMetricsManager = new MetricsManager(key, config, mTimeBaseSec, mUidMap, mAnomalyAlarmMonitor, mPeriodicAlarmMonitor); - auto it = mMetricsManagers.find(key); - if (it == mMetricsManagers.end() && mMetricsManagers.size() > StatsdStats::kMaxConfigCount) { - ALOGE("Can't accept more configs!"); - return; - } if (newMetricsManager->isConfigValid()) { mUidMap->OnConfigUpdated(key); @@ -213,7 +208,6 @@ void StatsLogProcessor::OnConfigUpdated(const ConfigKey& key, const StatsdConfig mUidMap->addListener(newMetricsManager.get()); } mMetricsManagers[key] = newMetricsManager; - // Why doesn't this work? mMetricsManagers.insert({key, std::move(newMetricsManager)}); VLOG("StatsdConfig valid"); } else { // If there is any error in the config, don't use it. diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp index aece141c37a80..ff2509149cf83 100644 --- a/cmds/statsd/src/config/ConfigManager.cpp +++ b/cmds/statsd/src/config/ConfigManager.cpp @@ -14,9 +14,13 @@ * limitations under the License. */ +#define DEBUG false // STOPSHIP if true +#include "Log.h" + #include "config/ConfigManager.h" #include "storage/StorageManager.h" +#include "guardrail/StatsdStats.h" #include "stats_util.h" #include @@ -68,24 +72,37 @@ void ConfigManager::UpdateConfig(const ConfigKey& key, const StatsdConfig& confi { lock_guard lock(mMutex); - auto it = mConfigs.find(key); - const int numBytes = config.ByteSize(); vector buffer(numBytes); config.SerializeToArray(&buffer[0], numBytes); - const bool isDuplicate = - it != mConfigs.end() && - StorageManager::hasIdenticalConfig(key, buffer); + auto uidIt = mConfigs.find(key.GetUid()); + // GuardRail: Limit the number of configs per uid. + if (uidIt != mConfigs.end()) { + auto it = uidIt->second.find(key); + if (it == uidIt->second.end() && + uidIt->second.size() >= StatsdStats::kMaxConfigCountPerUid) { + ALOGE("ConfigManager: uid %d has exceeded the config count limit", key.GetUid()); + return; + } + } - // Update saved file on disk. We still update timestamp of file when - // there exists a duplicate configuration to avoid garbage collection. + // Check if it's a duplicate config. + if (uidIt != mConfigs.end() && uidIt->second.find(key) != uidIt->second.end() && + StorageManager::hasIdenticalConfig(key, buffer)) { + // This is a duplicate config. + ALOGI("ConfigManager This is a duplicate config %s", key.ToString().c_str()); + // Update saved file on disk. We still update timestamp of file when + // there exists a duplicate configuration to avoid garbage collection. + update_saved_configs_locked(key, buffer, numBytes); + return; + } + + // Update saved file on disk. update_saved_configs_locked(key, buffer, numBytes); - if (isDuplicate) return; - - // Add to set - mConfigs.insert(key); + // Add to set. + mConfigs[key.GetUid()].insert(key); for (sp listener : mListeners) { broadcastList.push_back(listener); @@ -113,11 +130,10 @@ void ConfigManager::RemoveConfig(const ConfigKey& key) { { lock_guard lock(mMutex); - auto it = mConfigs.find(key); - if (it != mConfigs.end()) { + auto uidIt = mConfigs.find(key.GetUid()); + if (uidIt != mConfigs.end() && uidIt->second.find(key) != uidIt->second.end()) { // Remove from map - mConfigs.erase(it); - + uidIt->second.erase(key); for (sp listener : mListeners) { broadcastList.push_back(listener); } @@ -150,18 +166,20 @@ void ConfigManager::RemoveConfigs(int uid) { { lock_guard lock(mMutex); - for (auto it = mConfigs.begin(); it != mConfigs.end();) { + auto uidIt = mConfigs.find(uid); + if (uidIt == mConfigs.end()) { + return; + } + + for (auto it = uidIt->second.begin(); it != uidIt->second.end(); ++it) { // Remove from map - if (it->GetUid() == uid) { remove_saved_configs(*it); removed.push_back(*it); mConfigReceivers.erase(*it); - it = mConfigs.erase(it); - } else { - it++; - } } + mConfigs.erase(uidIt); + for (sp listener : mListeners) { broadcastList.push_back(listener); } @@ -182,17 +200,16 @@ void ConfigManager::RemoveAllConfigs() { { lock_guard lock(mMutex); - - for (auto it = mConfigs.begin(); it != mConfigs.end();) { - // Remove from map - removed.push_back(*it); - auto receiverIt = mConfigReceivers.find(*it); - if (receiverIt != mConfigReceivers.end()) { - mConfigReceivers.erase(*it); + for (auto uidIt = mConfigs.begin(); uidIt != mConfigs.end();) { + for (auto it = uidIt->second.begin(); it != uidIt->second.end();) { + // Remove from map + removed.push_back(*it); + it = uidIt->second.erase(it); } - it = mConfigs.erase(it); + uidIt = mConfigs.erase(uidIt); } + mConfigReceivers.clear(); for (sp listener : mListeners) { broadcastList.push_back(listener); } @@ -211,8 +228,10 @@ vector ConfigManager::GetAllConfigKeys() const { lock_guard lock(mMutex); vector ret; - for (auto it = mConfigs.cbegin(); it != mConfigs.cend(); ++it) { - ret.push_back(*it); + for (auto uidIt = mConfigs.cbegin(); uidIt != mConfigs.cend(); ++uidIt) { + for (auto it = uidIt->second.cbegin(); it != uidIt->second.cend(); ++it) { + ret.push_back(*it); + } } return ret; } @@ -231,13 +250,15 @@ const sp ConfigManager::GetConfigReceiver(const ConfigKey& key void ConfigManager::Dump(FILE* out) { lock_guard lock(mMutex); - fprintf(out, "CONFIGURATIONS (%d)\n", (int)mConfigs.size()); + fprintf(out, "CONFIGURATIONS\n"); fprintf(out, " uid name\n"); - for (const auto& key : mConfigs) { - fprintf(out, " %6d %lld\n", key.GetUid(), (long long)key.GetId()); - auto receiverIt = mConfigReceivers.find(key); - if (receiverIt != mConfigReceivers.end()) { - fprintf(out, " -> received by PendingIntent as binder\n"); + for (auto uidIt = mConfigs.cbegin(); uidIt != mConfigs.cend(); ++uidIt) { + for (auto it = uidIt->second.cbegin(); it != uidIt->second.cend(); ++it) { + fprintf(out, " %6d %lld\n", it->GetUid(), (long long)it->GetId()); + auto receiverIt = mConfigReceivers.find(*it); + if (receiverIt != mConfigReceivers.end()) { + fprintf(out, " -> received by PendingIntent as binder\n"); + } } } } diff --git a/cmds/statsd/src/config/ConfigManager.h b/cmds/statsd/src/config/ConfigManager.h index 9a38188a120c3..611c34250a385 100644 --- a/cmds/statsd/src/config/ConfigManager.h +++ b/cmds/statsd/src/config/ConfigManager.h @@ -36,9 +36,6 @@ StatsdConfig build_fake_config(); /** * Keeps track of which configurations have been set from various sources. - * - * TODO: Store the configs persistently too. - * TODO: Dump method for debugging. */ class ConfigManager : public virtual android::RefBase { public: @@ -125,9 +122,9 @@ private: void remove_saved_configs(const ConfigKey& key); /** - * Config keys that have been set. + * Maps from uid to the config keys that have been set. */ - std::set mConfigs; + std::map> mConfigs; /** * Each config key can be subscribed by up to one receiver, specified as IBinder from diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h index 767588808d45b..7f8755b8c1e06 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.h +++ b/cmds/statsd/src/guardrail/StatsdStats.h @@ -78,17 +78,17 @@ public: ~StatsdStats(){}; // TODO: set different limit if the device is low ram. - const static int kDimensionKeySizeSoftLimit = 300; - const static int kDimensionKeySizeHardLimit = 500; + const static int kDimensionKeySizeSoftLimit = 500; + const static int kDimensionKeySizeHardLimit = 800; // Per atom dimension key size limit static const std::map> kAtomDimensionKeySizeLimitMap; - const static int kMaxConfigCount = 10; + const static int kMaxConfigCountPerUid = 10; const static int kMaxAlertCountPerConfig = 100; - const static int kMaxConditionCountPerConfig = 200; - const static int kMaxMetricCountPerConfig = 300; - const static int kMaxMatcherCountPerConfig = 500; + const static int kMaxConditionCountPerConfig = 300; + const static int kMaxMetricCountPerConfig = 1000; + const static int kMaxMatcherCountPerConfig = 800; // The max number of old config stats we keep. const static int kMaxIceBoxSize = 20;