wcarlson5 commented on a change in pull request #9273:
URL: https://github.com/apache/kafka/pull/9273#discussion_r498507007
##########
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:
If the client is in error we should close these threads, for the same
reason we are closing them for this kip. So might as well close them now.
This is also ensures they are closed on all clients after a shutdown
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -364,6 +368,62 @@ public void setUncaughtExceptionHandler(final
Thread.UncaughtExceptionHandler eh
}
}
+ /**
+ * Set the handler invoked when a {@link
StreamsConfig#NUM_STREAM_THREADS_CONFIG internal thread} abruptly
+ * terminates due to an uncaught exception.
+ *
+ * @param eh the uncaught exception handler of type {@link
StreamsUncaughtExceptionHandler} for all internal threads; {@code null} deletes
the current handler
+ * @throws IllegalStateException if this {@code KafkaStreams} instance is
not in state {@link State#CREATED CREATED}.
+ */
+ public void setUncaughtExceptionHandler(final
StreamsUncaughtExceptionHandler eh) {
+ final StreamsUncaughtExceptionHandler handler = exception ->
handleStreamsUncaughtException(exception, eh);
+ synchronized (stateLock) {
+ if (state == State.CREATED) {
+ for (final StreamThread thread : threads) {
+ if (eh != null) {
+ thread.setStreamsUncaughtExceptionHandler(handler);
+ } else {
+ final StreamsUncaughtExceptionHandler defaultHandler =
exception ->
+
StreamsUncaughtExceptionHandler.StreamsUncaughtExceptionHandlerResponse.SHUTDOWN_STREAM_THREAD;
+
thread.setStreamsUncaughtExceptionHandler(defaultHandler);
+ }
+ }
+ } else {
+ throw new IllegalStateException("Can only set
UncaughtExceptionHandler in CREATED state. " +
+ "Current state is: " + state);
+ }
+ }
+ }
+
+ private
StreamsUncaughtExceptionHandler.StreamsUncaughtExceptionHandlerResponse
handleStreamsUncaughtException(final Exception e,
+
final StreamsUncaughtExceptionHandler
streamsUncaughtExceptionHandler) {
+ final
StreamsUncaughtExceptionHandler.StreamsUncaughtExceptionHandlerResponse action
= streamsUncaughtExceptionHandler.handle(e);
+ switch (action) {
+ case SHUTDOWN_STREAM_THREAD:
+ log.error("Encountered the following exception during
processing " +
+ "and the thread is going to shut down: ", e);
+ break;
+ case REPLACE_STREAM_THREAD:
+ log.error("Encountered the following exception during
processing " +
+ "and the the stream thread will be replaced: ", e);
//TODO: add then remove, wait until 663 is merged
Review comment:
need to wait to @cadonna to merge 663 before this is added. Can do a
partial implantation then add later though
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamThread.java
##########
@@ -550,6 +561,10 @@ void runLoop() {
// until the rebalance is completed before we close and commit the
tasks
while (isRunning() || taskManager.isRebalanceInProgress()) {
try {
+ if (shutdownRequested.get()) {
+ sendShutdownRequest(shutdownTypeRequested);
+ return;
Review comment:
if we are shutting down may as well start now instead of waiting for the
rebalance. this will stop every thread
----------------------------------------------------------------
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]
