C0urante commented on a change in pull request #10503:
URL: https://github.com/apache/kafka/pull/10503#discussion_r609882468
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerTask.java
##########
@@ -185,8 +185,15 @@ private void doRun() throws InterruptedException {
execute();
} catch (Throwable t) {
- log.error("{} Task threw an uncaught and unrecoverable exception.
Task is being killed and will not recover until manually restarted", this, t);
- throw t;
+ if (!stopping) {
+ log.error("{} Task threw an uncaught and unrecoverable
exception. Task is being killed and will not recover until manually restarted",
this, t);
+ throw t;
+ } else {
+ log.warn("{} During stop, task threw an uncaught and
unrecoverable exception: {}.", this, t.getMessage());
Review comment:
Nit 1: "During stop" may be interpreted as "During an invocation of
`Task::stop`", which isn't necessarily the case here if, for example, shutdown
is scheduled but the task fails during a call to `put` or `poll` before `stop`
can even be invoked.
Nit 2: Also, this might be more of a personal thing, but I'm wondering if
"unrecoverable" really adds much here since, even if the task were to somehow
"recover", it'd still just shut down.
Nit 3: I think it's still valuable to preserve the entire stack trace
instead of only revealing with `DEBUG`-level logging. Any time a task throws an
uncaught exception, it should be cause for concern, as there may be issues with
the quality of the connector, the health of the Kafka cluster, or the
configuration of the worker. And since task restarts have to be manually
triggered, it doesn't seem likely that this'd lead to logs being flooded with
stack traces.
WDYT about something like `log.warn("{} After being scheduled for shutdown,
task threw an uncaught exception.", this, t);`?
--
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:
us...@infra.apache.org
