From cfdf67144c24f8ccbb30d883111f6bf105db6084 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Wed, 12 Aug 2020 01:57:49 -0700 Subject: [PATCH] Change condition init to return current condition To allow initialization of conditions to work for both config creation and update, have init always fill the conditionCache with the current condition. For sliced dimensions, it should compute an OR over all dimensions, returning true if any of the dimensions are true. We can use this for setting the initial value of new combination conditions that depend on existing initial conditions, or for new metrics that depend on existing conditions. Test: atest statsd_test. Existing tests pass, will add more for the update path in the update for condition cl. Bug: 162322841 Change-Id: Ib48479269828f2e071c5c46aba1129d52d4725de --- .../condition/CombinationConditionTracker.cpp | 29 ++++++++++++++----- .../condition/CombinationConditionTracker.h | 2 +- cmds/statsd/src/condition/ConditionTracker.h | 8 +++-- .../src/condition/SimpleConditionTracker.cpp | 9 ++++-- .../src/condition/SimpleConditionTracker.h | 2 +- .../parsing_utils/metrics_manager_util.cpp | 3 +- 6 files changed, 35 insertions(+), 18 deletions(-) diff --git a/cmds/statsd/src/condition/CombinationConditionTracker.cpp b/cmds/statsd/src/condition/CombinationConditionTracker.cpp index 7a1ece94fd3e0..3b65f8225ee95 100644 --- a/cmds/statsd/src/condition/CombinationConditionTracker.cpp +++ b/cmds/statsd/src/condition/CombinationConditionTracker.cpp @@ -38,9 +38,23 @@ bool CombinationConditionTracker::init(const vector& allConditionConf const vector>& allConditionTrackers, const unordered_map& conditionIdIndexMap, vector& stack, - vector& initialConditionCache) { + vector& conditionCache) { VLOG("Combination predicate init() %lld", (long long)mConditionId); if (mInitialized) { + // All the children are guaranteed to be initialized, but the recursion is needed to + // fill the conditionCache properly, since another combination condition or metric + // might rely on this. The recursion is needed to compute the current condition. + + // Init is called instead of isConditionMet so that the ConditionKey can be filled with the + // default key for sliced conditions, since we do not know all indirect descendants here. + for (const int childIndex : mChildren) { + if (conditionCache[childIndex] == ConditionState::kNotEvaluated) { + allConditionTrackers[childIndex]->init(allConditionConfig, allConditionTrackers, + conditionIdIndexMap, stack, conditionCache); + } + } + conditionCache[mIndex] = + evaluateCombinationCondition(mChildren, mLogicalOperation, conditionCache); return true; } @@ -74,9 +88,8 @@ bool CombinationConditionTracker::init(const vector& allConditionConf return false; } - bool initChildSucceeded = - childTracker->init(allConditionConfig, allConditionTrackers, conditionIdIndexMap, - stack, initialConditionCache); + bool initChildSucceeded = childTracker->init(allConditionConfig, allConditionTrackers, + conditionIdIndexMap, stack, conditionCache); if (!initChildSucceeded) { ALOGW("Child initialization failed %lld ", (long long)child); @@ -96,10 +109,10 @@ bool CombinationConditionTracker::init(const vector& allConditionConf childTracker->getAtomMatchingTrackerIndex().end()); } - mUnSlicedPartCondition = evaluateCombinationCondition(mUnSlicedChildren, mLogicalOperation, - initialConditionCache); - initialConditionCache[mIndex] = - evaluateCombinationCondition(mChildren, mLogicalOperation, initialConditionCache); + mUnSlicedPartCondition = + evaluateCombinationCondition(mUnSlicedChildren, mLogicalOperation, conditionCache); + conditionCache[mIndex] = + evaluateCombinationCondition(mChildren, mLogicalOperation, conditionCache); // unmark this node in the recursion stack. stack[mIndex] = false; diff --git a/cmds/statsd/src/condition/CombinationConditionTracker.h b/cmds/statsd/src/condition/CombinationConditionTracker.h index 39ff0ab03266d..a7fac3deaabec 100644 --- a/cmds/statsd/src/condition/CombinationConditionTracker.h +++ b/cmds/statsd/src/condition/CombinationConditionTracker.h @@ -33,7 +33,7 @@ public: bool init(const std::vector& allConditionConfig, const std::vector>& allConditionTrackers, const std::unordered_map& conditionIdIndexMap, std::vector& stack, - std::vector& initialConditionCache) override; + std::vector& conditionCache) override; void evaluateCondition(const LogEvent& event, const std::vector& eventMatcherValues, diff --git a/cmds/statsd/src/condition/ConditionTracker.h b/cmds/statsd/src/condition/ConditionTracker.h index 9da1af427e5f7..4e1253506be76 100644 --- a/cmds/statsd/src/condition/ConditionTracker.h +++ b/cmds/statsd/src/condition/ConditionTracker.h @@ -46,17 +46,19 @@ public: // Initialize this ConditionTracker. This initialization is done recursively (DFS). It can also // be done in the constructor, but we do it separately because (1) easy to return a bool to // indicate whether the initialization is successful. (2) makes unit test easier. + // This function can also be called on config updates, in which case it does nothing other than + // fill the condition cache with the current condition. // allConditionConfig: the list of all Predicate config from statsd_config. // allConditionTrackers: the list of all ConditionTrackers (this is needed because we may also // need to call init() on children conditions) // conditionIdIndexMap: the mapping from condition id to its index. // stack: a bit map to keep track which nodes have been visited on the stack in the recursion. - // initialConditionCache: tracks initial conditions of all ConditionTrackers. + // conditionCache: tracks initial conditions of all ConditionTrackers. returns the + // current condition if called on a config update. virtual bool init(const std::vector& allConditionConfig, const std::vector>& allConditionTrackers, const std::unordered_map& conditionIdIndexMap, - std::vector& stack, - std::vector& initialConditionCache) = 0; + std::vector& stack, std::vector& conditionCache) = 0; // evaluate current condition given the new event. // event: the new log event diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.cpp b/cmds/statsd/src/condition/SimpleConditionTracker.cpp index efb4d4989425e..f45759b6a77ef 100644 --- a/cmds/statsd/src/condition/SimpleConditionTracker.cpp +++ b/cmds/statsd/src/condition/SimpleConditionTracker.cpp @@ -95,11 +95,14 @@ SimpleConditionTracker::~SimpleConditionTracker() { bool SimpleConditionTracker::init(const vector& allConditionConfig, const vector>& allConditionTrackers, const unordered_map& conditionIdIndexMap, - vector& stack, - vector& initialConditionCache) { + vector& stack, vector& conditionCache) { // SimpleConditionTracker does not have dependency on other conditions, thus we just return // if the initialization was successful. - initialConditionCache[mIndex] = mInitialValue; + ConditionKey conditionKey; + if (mSliced) { + conditionKey[mConditionId] = DEFAULT_DIMENSION_KEY; + } + isConditionMet(conditionKey, allConditionTrackers, mSliced, conditionCache); return mInitialized; } diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.h b/cmds/statsd/src/condition/SimpleConditionTracker.h index ea7f87bde2b80..1a9e35e38207a 100644 --- a/cmds/statsd/src/condition/SimpleConditionTracker.h +++ b/cmds/statsd/src/condition/SimpleConditionTracker.h @@ -38,7 +38,7 @@ public: bool init(const std::vector& allConditionConfig, const std::vector>& allConditionTrackers, const std::unordered_map& conditionIdIndexMap, std::vector& stack, - std::vector& initialConditionCache) override; + std::vector& conditionCache) override; void evaluateCondition(const LogEvent& event, const std::vector& eventMatcherValues, diff --git a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp index daf67e93557c7..e40fbdb250f16 100644 --- a/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/parsing_utils/metrics_manager_util.cpp @@ -303,8 +303,7 @@ bool initConditions(const ConfigKey& key, const StatsdConfig& config, const int conditionTrackerCount = config.predicate_size(); conditionConfigs.reserve(conditionTrackerCount); allConditionTrackers.reserve(conditionTrackerCount); - initialConditionCache.reserve(conditionTrackerCount); - std::fill(initialConditionCache.begin(), initialConditionCache.end(), ConditionState::kUnknown); + initialConditionCache.assign(conditionTrackerCount, ConditionState::kNotEvaluated); for (int i = 0; i < conditionTrackerCount; i++) { const Predicate& condition = config.predicate(i);