gharris1727 commented on code in PR #16914:
URL: https://github.com/apache/kafka/pull/16914#discussion_r1722383520
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStreamThread.java:
##########
@@ -453,11 +453,15 @@ private void closeStateConsumer(final StateConsumer
stateConsumer, final boolean
@Override
public synchronized void start() {
super.start();
- while (stillInitializing()) {
- Utils.sleep(1);
- if (startupException != null) {
- throw startupException;
- }
+ try {
+ initializationLatch.await();
+ } catch (final InterruptedException e) {
+ currentThread().interrupt();
+ throw new IllegalStateException("Thread was interrupted during
initialization", e);
Review Comment:
This is a "new" exception, as the previous code would have just spun calling
Utils.sleep and continuously throwing InterruptedExceptions. It's more
informative than the other IllegalStateException thrown here, so this seems
reasonable.
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStreamThread.java:
##########
@@ -175,6 +175,12 @@ private void setState(final State newState) {
}
state = newState;
+
+ // Question: Is this a good idea? Or should we spread the latch
countdown to finally blocks
+ // Count down the latch if transitioning from CREATED to any other
state
+ if (oldState == State.CREATED && newState != State.CREATED) {
+ initializationLatch.countDown();
+ }
Review Comment:
> The other alternative would be to count down in finally blocks across the
class.
It could make sense to have a finally block in the `initialize` method and
call `countDown` in `shutdown`, but the current implementation appears to have
reasonable semantics.
I have a weak feeling that counting down this latch is too much for the
setState method to be doing, but at the same time pushing the latch management
out into the calling code risks missing a (possibly future) call-site which
transitions away from CREATED.
I think i would leave this to see what the Streams folks think about it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
