cadonna commented on a change in pull request #9720:
URL: https://github.com/apache/kafka/pull/9720#discussion_r561734967
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -1165,11 +1151,21 @@ private Thread shutdownHelper(final boolean error) {
metrics.close();
if (!error) {
setState(State.NOT_RUNNING);
+ } else {
+ setState(State.ERROR);
}
}, "kafka-streams-close-thread");
}
private boolean close(final long timeoutMs) {
+ if (state == State.ERROR) {
+ log.info("Streams client is already in the terminal state ERROR,
all resources are closed and the client has stopped.");
+ return false;
Review comment:
As far as I can see, where the return value is used the javadoc says
```
true if all threads were successfully stopped, false if the timeout was
reached.
```
Since all threads were successfully stopped, I would return `true`. We
clearly document that `ERROR` is a terminal state, so I do not see why somebody
should wait for `NOT_RUNNING` when the client is in `ERROR` and `close()`
returns `true`.
----------------------------------------------------------------
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
