guozhangwang commented on pull request #11455:
URL: https://github.com/apache/kafka/pull/11455#issuecomment-959833451
@cadonna I had some discussions with @mjsax about this, and my main
motivation comes from the old logic before the latest PR that modifies the
instance level diagram and introduced `closeToError`:
```
if (newState == GlobalStreamThread.State.RUNNING) {
maybeSetRunning();
} else if (newState == GlobalStreamThread.State.DEAD) {
if (setState(State.ERROR)) {
log.error("Global thread has died. The instance
will be in error state and should be closed.");
}
}
```
I think as you brought up, the thread state transiting from PENDING_SHUTDOWN
to DEAD cannot tell whether it is shutting down normally or abnormally; only at
the instance level state we can differentiate, since now that transition
diagram has PENDING_ERROR -> ERROR and PENDING_SHUTDOWN -> NOT_RUNNING.
So if it is shutdown normally, then the instance level state should already
been at `PENDING_SHUTDOWN`, so it should not be able to transit to
`PENDING_ERROR` anymore; if it is shutdown abnormally then, the instance level
state could be at `RUNNING / REBALANCING` in which case transit to
`PENDING_ERROR` should be fine.
On the other hand, before we call `setState(DEAD)` in global thread, if it
is from the exception case, the handler should already have been called indeed,
in which we would trigger `closeToError`, so there's no need to call
`closeToError` again. In that sense, I feel we can still safely remove the
whole block of
```
else if (newState == GlobalStreamThread.State.DEAD) {
log.error("Global thread has died. The streams
application or client will now close to ERROR.");
closeToError();
}
```
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
