vvcephei commented on a change in pull request #9273:
URL: https://github.com/apache/kafka/pull/9273#discussion_r499842547
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -436,6 +496,8 @@ private void maybeSetError() {
}
if (setState(State.ERROR)) {
+ metrics.close();
Review comment:
It certainly might. I'm just wary of how far into the uncanny valley
we're going here. Streams is going to be put into a state that's very similar
to the one that `close()` produces, but not identical. What will then happen
when they _do_ call close? What will happen when we realize that something else
needs to be done as part of closing the instance (will we even remember that we
should consider doing it here as well)?
OTOH, we could instead change direction on the "error-vs-shutdown" debase
and just make all these methods call `close(ZERO)` instead. Then, the _real_
close method will be invoked, and Streams will go through a well-known
transition through `PENDING_SHUTDOWN` to `NOT_RUNNING`.
It would then be a problem for a later date (after KIP-663) if someone
wanted to request that instead the app should stop all running threads so they
can manually call "addThread" later.
----------------------------------------------------------------
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]
