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/04/02 17:39:14 UTC
[GitHub] [incubator-pinot] xiaohui-sun opened a new pull request #5207: [TE] Retain the original last success task execution time if we can't…
xiaohui-sun opened a new pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207
… find success tasks
There are several alerts that failed for more than 30 days, and thus we can't get the last success task execution from this window.
However we should keep the original last success task execution 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] jihaozh commented on a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207#discussion_r402539114
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/datalayer/pojo/DetectionConfigBean.java
##########
@@ -162,6 +162,12 @@ public DetectionHealth getHealth() {
}
public void setHealth(DetectionHealth health) {
+ // If there is no success execution in the current window then the last success task execution time would be -1L.
+ // We should keep the original last success task execution time if we have.
+ long lastSuccessTaskExecutionTime = this.health.getDetectionTaskStatus().getLastSuccessTaskExecutionTime();
Review comment:
Shall we put this logic in `MonitorTaskRunner` where the detection health is updated?
----------------------------------------------------------------
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 a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207#discussion_r403642871
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/health/DetectionTaskStatus.java
##########
@@ -46,10 +46,14 @@
// the time stamp of last successfully finishing task
@JsonProperty
- private final Long lastTaskExecutionTime;
+ private Long lastSuccessTaskExecutionTime;
Review comment:
Reverted.
----------------------------------------------------------------
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 #5207: [TE] Retain the original last success task execution time if we can't…
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207#discussion_r402536249
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/health/DetectionTaskStatus.java
##########
@@ -46,10 +46,14 @@
// the time stamp of last successfully finishing task
@JsonProperty
- private final Long lastTaskExecutionTime;
+ private Long lastSuccessTaskExecutionTime;
Review comment:
We may need to keep the name, otherwise, the existing JSON blobs in the DB won't be able to deserialize back to the object.
----------------------------------------------------------------
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 a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
Posted by GitBox <gi...@apache.org>.
xiaohui-sun commented on a change in pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207#discussion_r403642882
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/datalayer/pojo/DetectionConfigBean.java
##########
@@ -162,6 +162,12 @@ public DetectionHealth getHealth() {
}
public void setHealth(DetectionHealth health) {
+ // If there is no success execution in the current window then the last success task execution time would be -1L.
+ // We should keep the original last success task execution time if we have.
+ long lastSuccessTaskExecutionTime = this.health.getDetectionTaskStatus().getLastSuccessTaskExecutionTime();
Review comment:
Makes sense! I changed that. Can you review again? @jihaozh
----------------------------------------------------------------
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 merged pull request #5207: [TE] Retain the original last success task execution time if we can't…
Posted by GitBox <gi...@apache.org>.
xiaohui-sun merged pull request #5207: [TE] Retain the original last success task execution time if we can't…
URL: https://github.com/apache/incubator-pinot/pull/5207
----------------------------------------------------------------
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