FrankChen021 commented on code in PR #20072:
URL: https://github.com/apache/druid/pull/20072#discussion_r3813153453
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -2060,13 +2060,25 @@ public void runInternal()
logDebugReport();
}
catch (Exception e) {
- stateManager.recordThrowableEvent(e);
- if (e instanceof StreamException) {
- // When a StreamException is thrown, the error message is more useful
than the stack trace in telling what's wrong.
- log.makeAlert("Exception in supervisor run loop for supervisor[%s] for
dataSource[%s]: [%s]",
- supervisorId, dataSource, e.getMessage()).emit();
+ if (e instanceof ExecutionException || e instanceof
InterruptedException) {
Review Comment:
[P2] Recoverable exits are counted as successful runs
Skipping recordThrowableEvent leaves currentRunSuccessful true, while
finally calls markRunFinished(). An aborted first run can therefore set
atLeastOneSuccessfulRun and transition to RUNNING, bypassing first-run
stream-failure handling on the next iteration.
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -2060,13 +2060,25 @@ public void runInternal()
logDebugReport();
}
catch (Exception e) {
- stateManager.recordThrowableEvent(e);
- if (e instanceof StreamException) {
- // When a StreamException is thrown, the error message is more useful
than the stack trace in telling what's wrong.
- log.makeAlert("Exception in supervisor run loop for supervisor[%s] for
dataSource[%s]: [%s]",
- supervisorId, dataSource, e.getMessage()).emit();
+ if (e instanceof ExecutionException || e instanceof
InterruptedException) {
Review Comment:
[P1] Task failures still reach unhealthy-state accounting
Per-task failures are converted to Either.error and recorded inside
checkpoint and pause callbacks, so they never reach this outer catch. Checked
errors can also be wrapped as RuntimeException. Classify recoverable failures
at their recording sites and add regression coverage.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
