guozhangwang commented on a change in pull request #10646:
URL: https://github.com/apache/kafka/pull/10646#discussion_r633858091
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/GlobalStateManagerImpl.java
##########
@@ -174,13 +176,15 @@ public void registerStore(final StateStore store, final
StateRestoreCallback sta
throw new IllegalArgumentException(String.format("Trying to
register store %s that is not a known global store", store.name()));
}
+ // register the store first, so that if later an exception is thrown
then eventually while we call `close`
Review comment:
Okay, I think I got what we were discussing now. Originally I'm thinking
that since these conditions should never happen --- because in the topology
when we `add state stores` we already check if the store names have existed or
not, and hence we should never add two stores with the same name --- if it ever
happens we would always treat it as fatal and crash stop immediately.
On the higher level, I think we should NOT allow users to handle illegal-s/a
themselves and hence ever possibly to treat them not as fatal, but obviously
today we do not enforce that.
So I think we can have two options here: 1) in the lower level hierarchy
like state manager here, try to stop the stores when hitting an illegal-s/a; 2)
on the higher level hierarchy as in stream thread, we enforce "stop app" on
illegal-s/a. I'm a bit leaning towards 2) here but would love to hear other
opinions.
--
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
