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/03/13 23:03:01 UTC
[GitHub] [incubator-pinot] akshayrai opened a new pull request #5152:
[TE][subscription] update subscription watermarks to use anomaly create
time instead of end time
akshayrai opened a new pull request #5152: [TE][subscription] update subscription watermarks to use anomaly create time instead of end time
URL: https://github.com/apache/incubator-pinot/pull/5152
Problem Statement:
The current subscription watermarks are designed to notify an anomaly only once (even if merged) and we maintain this by keeping track of the last notified anomaly end time (watermark). However, the assumption here was that newer anomalies will always be detected on newer data (that is, newer anomalies can never have start time < watermark). This puts the restriction when dealing with backfilled data and also in the case of missing data where the actual deviation anomalies on the data might be detected later. This PR tries to remove this restriction by leveraging the anomaly create time in the watermark.
Proposed changes:
* Replace the anomaly end time with the anomaly create time in the vector clock.
* Remove highWaterMark field from subscription config - As of today, we maintain 2 watermarks, namely the last notified anomaly ID(highWaterMark) and the anomaly end time watermark(vector clocks) to ensure that each anomaly is notified only once. The main purpose of the highWaterMark is to filter out merged anomalies from the time window. This is no longer required once we start relying on the anomaly create time.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on issue #5152:
[TE][subscription] update subscription watermarks to use anomaly create
time instead of end time
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on issue #5152: [TE][subscription] update subscription watermarks to use anomaly create time instead of end time
URL: https://github.com/apache/incubator-pinot/pull/5152#issuecomment-600663181
Thanks for this fix.
Can you also update needNotification() in DetectionAlertJob class and use the create time?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request
#5152: [TE][subscription] update subscription watermarks to use anomaly
create time instead of end time
Posted by GitBox <gi...@apache.org>.
akshayrai commented on a change in pull request #5152: [TE][subscription] update subscription watermarks to use anomaly create time instead of end time
URL: https://github.com/apache/incubator-pinot/pull/5152#discussion_r393910889
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/SubscriptionConfigValidator.java
##########
@@ -90,10 +90,6 @@ public void validateConfig(DetectionAlertConfigDTO alertConfig) throws IllegalAr
*/
@Override
public void validateYaml(Map<String, Object> config) throws IllegalArgumentException {
- // Make sure recipients are configured at the right place
- Preconditions.checkArgument(!config.containsKey(PROP_RECIPIENTS),
Review comment:
That's already remove in another PR and merged. I need to rebase the changes.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#5152: [TE][subscription] update subscription watermarks to use anomaly
create time instead of end time
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5152: [TE][subscription] update subscription watermarks to use anomaly create time instead of end time
URL: https://github.com/apache/incubator-pinot/pull/5152#discussion_r393898983
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/SubscriptionConfigValidator.java
##########
@@ -90,10 +90,6 @@ public void validateConfig(DetectionAlertConfigDTO alertConfig) throws IllegalAr
*/
@Override
public void validateYaml(Map<String, Object> config) throws IllegalArgumentException {
- // Make sure recipients are configured at the right place
- Preconditions.checkArgument(!config.containsKey(PROP_RECIPIENTS),
Review comment:
Are we removing this? Seems unrelated.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #5152:
[TE][subscription] update subscription watermarks to use anomaly create
time instead of end time
Posted by GitBox <gi...@apache.org>.
akshayrai merged pull request #5152: [TE][subscription] update subscription watermarks to use anomaly create time instead of end time
URL: https://github.com/apache/incubator-pinot/pull/5152
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org