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?
   
   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:
us...@infra.apache.org


Reply via email to