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);