You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/25 18:29:51 UTC
[GitHub] [incubator-pinot] vincentchenjl opened a new pull request #5920: [TE] only merge same trend in ChildKeepingMerge
vincentchenjl opened a new pull request #5920:
URL: https://github.com/apache/incubator-pinot/pull/5920
This PR is to fix the issue that ChildKeepMergeWrapper merges anomalies with different trend. After this PR, the wrapper will only merge anomalies with same trend, e.g anomalies with UP trend, meaning current value higher than predicted value.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5920: [TE] only merge same trend in ChildKeepingMerge
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5920:
URL: https://github.com/apache/incubator-pinot/pull/5920#discussion_r477835580
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java
##########
@@ -91,10 +93,13 @@ public ChildKeepingMergeWrapper(DataProvider provider, DetectionConfigDTO config
if (anomaly.getProperties().containsKey(PROP_GROUP_KEY)) {
groupKey = anomaly.getProperties().get(PROP_GROUP_KEY);
}
-
+ String patternKey = "";
+ if (anomaly.getProperties().containsKey(PROP_PATTERN_KEY)) {
+ patternKey = anomaly.getProperties().get(PROP_PATTERN_KEY);
Review comment:
I think that a few rule-based detectors don't fill out the baseline value of `MergedAnomalyResult`, `ThresholdRuleDetector` for instance, so comparing them may not work for all detectors either.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai merged pull request #5920: [TE] only merge same trend in ChildKeepingMerge
Posted by GitBox <gi...@apache.org>.
akshayrai merged pull request #5920:
URL: https://github.com/apache/incubator-pinot/pull/5920
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #5920: [TE] only merge same trend in ChildKeepingMerge
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5920:
URL: https://github.com/apache/incubator-pinot/pull/5920#discussion_r477664239
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/ChildKeepingMergeWrapper.java
##########
@@ -91,10 +93,13 @@ public ChildKeepingMergeWrapper(DataProvider provider, DetectionConfigDTO config
if (anomaly.getProperties().containsKey(PROP_GROUP_KEY)) {
groupKey = anomaly.getProperties().get(PROP_GROUP_KEY);
}
-
+ String patternKey = "";
+ if (anomaly.getProperties().containsKey(PROP_PATTERN_KEY)) {
+ patternKey = anomaly.getProperties().get(PROP_PATTERN_KEY);
Review comment:
some anomalies don't have the pattern key. in that case, should it be deducted from the current and the baseline?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Ruoyingw commented on pull request #5920: [TE] only merge same trend in ChildKeepingMerge
Posted by GitBox <gi...@apache.org>.
Ruoyingw commented on pull request #5920:
URL: https://github.com/apache/incubator-pinot/pull/5920#issuecomment-680266648
lgtm. Thanks for the quick fix!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org