[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #4685: [TE] handle data insufficient exceptions in the detection pipeline

[GitBox <gi...@apache.org>]

vincentchenjl commented on a change in pull request #4685: [TE] handle data insufficient exceptions in the detection pipeline
URL: https://github.com/apache/incubator-pinot/pull/4685#discussion_r332662501
 
 

 ##########
 File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/algorithm/DimensionWrapper.java
 ##########
 @@ -278,22 +286,48 @@ public DetectionPipelineResult run() throws Exception {
         .collect(Collectors.toList());
   }
 
-  private void checkEarlyStop(long totalNestedMetrics, long successNestedMetrics, int i, Exception lastException) throws DetectionPipelineException {
+  private void checkEarlyStop(long totalNestedMetrics, long successNestedMetrics, int i, List<Exception> exceptions)
+      throws DetectionPipelineException {
     // if the first certain number of dimensions all failed, throw an exception
     if (i == EARLY_STOP_THRESHOLD && successNestedMetrics == 0) {
       throw new DetectionPipelineException(String.format(
           "Detection failed for first %d out of %d metric dimensions for monitoring window %d to %d, stop processing.",
-          i, totalNestedMetrics, this.getStartTime(), this.getEndTime()), lastException);
+          i, totalNestedMetrics, this.getStartTime(), this.getEndTime()), Iterables.getLast(exceptions));
     }
   }
 
-  private void checkNestedMetricsStatus(long totalNestedMetrics, long successNestedMetrics, Exception lastException)
-      throws DetectionPipelineException {
+  /**
+   * Check the exception for all nested metric and determine whether to fail the detection.
+   *
+   * This method will throw an exception to outer wrappers if the all nested metrics failed.
+   * UNLESS the exception for all nested metrics are failed by
+   * {@link DetectorDataInsufficientException} and multiple detector are configured to run,
+   * and some of them run successfully. In such case, this method will
+   * still allow the detection to finish and return the generated anomalies. Because the data insufficient exception
+   * thrown by one detector should not block other detectors's result.
+   *
+   * @param totalNestedMetrics the total number of nested metrics
+   * @param successNestedMetrics the successfully generated nested metrics
+   * @param exceptions the list of all exceptions
+   * @param predictions the prediction results generated, which can tell us whether there are detectors run successfully.
+   * @throws DetectionPipelineException the exception to throw
+   */
+  private void checkNestedMetricsStatus(long totalNestedMetrics, long successNestedMetrics, List<Exception> exceptions,
+      List<PredictionResult> predictions) throws DetectionPipelineException {
     // if all nested metrics failed, throw an exception
     if (successNestedMetrics == 0 && totalNestedMetrics > 0) {
-      throw new DetectionPipelineException(String.format(
-          "Detection failed for all nested dimensions for detection config id %d for monitoring window %d to %d.",
-          this.config.getId(), this.getStartTime(), this.getEndTime()), lastException);
+      // if all exceptions are caused by DetectorDataInsufficientException and
+      // there are other detectors run successfully, keep the detection running
 
 Review comment:
   Just curious what is the behavior if detectors in some dimensions throws other exceptions than `DetectorDataInsufficientException`. Do we still consider the detection is running successfully if one of the dimension fails due to other exceptions?

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