Update indices of preserved matchers

This cl updates mIndex and mChildren of preserved atom matchers across
config updates, since the index of the sp<> in mAllAtomMatchers can
change during a config update.

Test: atest statsd_test
Bug: 164282429
Change-Id: I7eb252439876d7330b9bba81e046fe3e89df6e5d
This commit is contained in:
Tej Singh
2020-08-17 20:27:17 -07:00
parent f7c04c3abc
commit 090ffd79c3
7 changed files with 81 additions and 6 deletions

View File

@@ -52,6 +52,15 @@ public:
const std::unordered_map<int64_t, int>& matcherMap,
std::vector<bool>& stack) = 0;
// Update appropriate state on config updates. Primarily, all indices need to be updated.
// This matcher and all of its children are guaranteed to be preserved across the update.
// matcher: the AtomMatcher proto from the config.
// index: the index of this matcher in mAllAtomMatchingTrackers.
// atomMatchingTrackerMap: map from matcher id to index in mAllAtomMatchingTrackers
virtual bool onConfigUpdated(
const AtomMatcher& matcher, const int index,
const std::unordered_map<int64_t, int>& atomMatchingTrackerMap) = 0;
// Called when a log event comes.
// event: the log event.
// allAtomMatchingTrackers: the list of all AtomMatchingTrackers. This is needed because the log
@@ -83,7 +92,7 @@ protected:
const int64_t mId;
// Index of this AtomMatchingTracker in MetricsManager's container.
const int mIndex;
int mIndex;
// Whether this AtomMatchingTracker has been properly initialized.
bool mInitialized;

View File

@@ -93,6 +93,23 @@ bool CombinationAtomMatchingTracker::init(
return true;
}
bool CombinationAtomMatchingTracker::onConfigUpdated(
const AtomMatcher& matcher, const int index,
const unordered_map<int64_t, int>& atomMatchingTrackerMap) {
mIndex = index;
mChildren.clear();
AtomMatcher_Combination combinationMatcher = matcher.combination();
for (const int64_t child : combinationMatcher.matcher()) {
const auto& pair = atomMatchingTrackerMap.find(child);
if (pair == atomMatchingTrackerMap.end()) {
ALOGW("Matcher %lld not found in the config", (long long)child);
return false;
}
mChildren.push_back(pair->second);
}
return true;
}
void CombinationAtomMatchingTracker::onLogEvent(
const LogEvent& event, const vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
vector<MatchingState>& matcherResults) {

View File

@@ -27,7 +27,7 @@ namespace os {
namespace statsd {
// Represents a AtomMatcher_Combination in the StatsdConfig.
class CombinationAtomMatchingTracker : public virtual AtomMatchingTracker {
class CombinationAtomMatchingTracker : public AtomMatchingTracker {
public:
CombinationAtomMatchingTracker(const int64_t& id, const int index, const uint64_t protoHash);
@@ -35,6 +35,9 @@ public:
const std::vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
const std::unordered_map<int64_t, int>& matcherMap, std::vector<bool>& stack);
bool onConfigUpdated(const AtomMatcher& matcher, const int index,
const std::unordered_map<int64_t, int>& atomMatchingTrackerMap) override;
~CombinationAtomMatchingTracker();
void onLogEvent(const LogEvent& event,
@@ -45,6 +48,8 @@ private:
LogicalOperation mLogicalOperation;
std::vector<int> mChildren;
FRIEND_TEST(ConfigUpdateTest, TestUpdateMatchers);
};
} // namespace statsd

View File

@@ -50,6 +50,14 @@ bool SimpleAtomMatchingTracker::init(const vector<AtomMatcher>& allAtomMatchers,
return mInitialized;
}
bool SimpleAtomMatchingTracker::onConfigUpdated(
const AtomMatcher& matcher, const int index,
const unordered_map<int64_t, int>& atomMatchingTrackerMap) {
mIndex = index;
// Do not need to update mMatcher since the matcher must be identical across the update.
return mInitialized;
}
void SimpleAtomMatchingTracker::onLogEvent(
const LogEvent& event, const vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
vector<MatchingState>& matcherResults) {

View File

@@ -28,7 +28,7 @@ namespace android {
namespace os {
namespace statsd {
class SimpleAtomMatchingTracker : public virtual AtomMatchingTracker {
class SimpleAtomMatchingTracker : public AtomMatchingTracker {
public:
SimpleAtomMatchingTracker(const int64_t& id, const int index, const uint64_t protoHash,
const SimpleAtomMatcher& matcher, const sp<UidMap>& uidMap);
@@ -40,6 +40,9 @@ public:
const std::unordered_map<int64_t, int>& matcherMap,
std::vector<bool>& stack) override;
bool onConfigUpdated(const AtomMatcher& matcher, const int index,
const std::unordered_map<int64_t, int>& atomMatchingTrackerMap) override;
void onLogEvent(const LogEvent& event,
const std::vector<sp<AtomMatchingTracker>>& allAtomMatchingTrackers,
std::vector<MatchingState>& matcherResults) override;

View File

@@ -148,8 +148,13 @@ bool updateAtomTrackers(const StatsdConfig& config, const sp<UidMap>& uidMap,
(long long)id);
return false;
}
const int oldIndex = oldAtomMatchingTrackerIt->second;
newAtomMatchingTrackers.push_back(oldAtomMatchingTrackers[oldIndex]);
const sp<AtomMatchingTracker>& tracker =
oldAtomMatchingTrackers[oldAtomMatchingTrackerIt->second];
if (!tracker->onConfigUpdated(matcherProtos[i], i, newAtomMatchingTrackerMap)) {
ALOGW("Config update failed for matcher %lld", (long long)id);
return false;
}
newAtomMatchingTrackers.push_back(tracker);
break;
}
case UPDATE_REPLACE: {

View File

@@ -23,6 +23,7 @@
#include <vector>
#include "frameworks/base/cmds/statsd/src/statsd_config.pb.h"
#include "src/matchers/CombinationAtomMatchingTracker.h"
#include "src/metrics/parsing_utils/metrics_manager_util.h"
#include "tests/statsd_test_util.h"
@@ -370,13 +371,40 @@ TEST_F(ConfigUpdateTest, TestUpdateMatchers) {
EXPECT_NE(oldAtomMatchingTrackers[oldAtomMatchingTrackerMap.at(combination2Id)],
newAtomMatchingTrackers[newAtomMatchingTrackerMap.at(combination2Id)]);
// Validation, make sure the matchers have the proper ids. Could do more checks here.
// Validation, make sure the matchers have the proper ids/indices. Could do more checks here.
EXPECT_EQ(newAtomMatchingTrackers[0]->getId(), combination3Id);
EXPECT_EQ(newAtomMatchingTrackers[0]->mIndex, 0);
EXPECT_EQ(newAtomMatchingTrackers[1]->getId(), simple2Id);
EXPECT_EQ(newAtomMatchingTrackers[1]->mIndex, 1);
EXPECT_EQ(newAtomMatchingTrackers[2]->getId(), combination2Id);
EXPECT_EQ(newAtomMatchingTrackers[2]->mIndex, 2);
EXPECT_EQ(newAtomMatchingTrackers[3]->getId(), simple1Id);
EXPECT_EQ(newAtomMatchingTrackers[3]->mIndex, 3);
EXPECT_EQ(newAtomMatchingTrackers[4]->getId(), simple4Id);
EXPECT_EQ(newAtomMatchingTrackers[4]->mIndex, 4);
EXPECT_EQ(newAtomMatchingTrackers[5]->getId(), combination1Id);
EXPECT_EQ(newAtomMatchingTrackers[5]->mIndex, 5);
// Verify child indices of Combination Matchers are correct.
CombinationAtomMatchingTracker* combinationTracker1 =
static_cast<CombinationAtomMatchingTracker*>(newAtomMatchingTrackers[5].get());
vector<int>* childMatchers = &combinationTracker1->mChildren;
EXPECT_EQ(childMatchers->size(), 1);
EXPECT_NE(std::find(childMatchers->begin(), childMatchers->end(), 3), childMatchers->end());
CombinationAtomMatchingTracker* combinationTracker2 =
static_cast<CombinationAtomMatchingTracker*>(newAtomMatchingTrackers[2].get());
childMatchers = &combinationTracker2->mChildren;
EXPECT_EQ(childMatchers->size(), 2);
EXPECT_NE(std::find(childMatchers->begin(), childMatchers->end(), 1), childMatchers->end());
EXPECT_NE(std::find(childMatchers->begin(), childMatchers->end(), 3), childMatchers->end());
CombinationAtomMatchingTracker* combinationTracker3 =
static_cast<CombinationAtomMatchingTracker*>(newAtomMatchingTrackers[0].get());
childMatchers = &combinationTracker3->mChildren;
EXPECT_EQ(childMatchers->size(), 2);
EXPECT_NE(std::find(childMatchers->begin(), childMatchers->end(), 1), childMatchers->end());
EXPECT_NE(std::find(childMatchers->begin(), childMatchers->end(), 4), childMatchers->end());
}
} // namespace statsd