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_r332664162
##########
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
+ if (exceptions.stream().allMatch(e -> e.getCause() instanceof
DetectorDataInsufficientException)
+ && predictions.size() != 0) {
Review comment:
I think that `predictions.size() == 0` will always true if
`successNestedMetrics == 0` is true. Please let me know if I miss something
here.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
