You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/04/26 07:28:17 UTC
[GitHub] [kafka] vitojeng opened a new pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
vitojeng opened a new pull request #10597:
URL: https://github.com/apache/kafka/pull/10597
follow-up #8200
KAFKA-5876's PR break into multiple parts, this PR is part 3 - apply StreamsNotStartedException
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-830978100
@ableegoldman Please take a look. :)
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r621436955
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -94,7 +94,14 @@ <h1>Upgrade Guide and API Changes</h1>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
<p>
- A new exception may be thrown from <code>KafkaStreams#store()</code>. If the specified store name does not exist in the topology, an UnknownStateStoreException will be thrown instead of the former InvalidStateStoreException. See <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-216%3A+IQ+should+throw+different+exceptions+for+different+errors">KIP-216</a> for more information.
+ Interactive Query may throw different new exceptions for different errors:
Review comment:
```suggestion
Interactive Queries may throw new exceptions for different errors:
```
##########
File path: docs/streams/upgrade-guide.html
##########
@@ -94,7 +94,14 @@ <h1>Upgrade Guide and API Changes</h1>
<h3><a id="streams_api_changes_300" href="#streams_api_changes_300">Streams API changes in 3.0.0</a></h3>
<p>
- A new exception may be thrown from <code>KafkaStreams#store()</code>. If the specified store name does not exist in the topology, an UnknownStateStoreException will be thrown instead of the former InvalidStateStoreException. See <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-216%3A+IQ+should+throw+different+exceptions+for+different+errors">KIP-216</a> for more information.
+ Interactive Query may throw different new exceptions for different errors:
+ </p>
+ <ul>
+ <li> <code>UnknownStateStoreException</code>: If the specified store name does not exist in the topology, an <code>UnknownStateStoreException</code> will be thrown instead of the former <code>InvalidStateStoreException</code>.</li>
+ <li> <code>StreamsNotStartedException</code>: If Streams state is not <code>REBALANCING</code> or <code>REBALANCING</code>, an <code>StreamsNotStartedException</code> will be thrown instead of the former <code>IllegalStateException</code>.</li>
Review comment:
```suggestion
<li> <code>StreamsNotStartedException</code>: If Streams state is not <code>REBALANCING</code> or <code>REBALANCING</code>, a <code>StreamsNotStartedException</code> will be thrown instead of the former <code>IllegalStateException</code>.</li>
```
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -344,7 +345,7 @@ private boolean isRunningOrRebalancing() {
private void validateIsRunningOrRebalancing() {
if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ throw new StreamsNotStartedException("KafkaStreams is not running. State is " + state + ".");
Review comment:
It seems a bit odd to throw a StreamsNotStartedException in case the KafkaStreams is not in RUNNING or REBALANCING. That seems to imply we would throw a a StreamsNotStartedException if the KafkaStreams had indeed been started, but then crashed or was closed to that the state is now one of the PENDING_{SHUTDOWN/ERROR} or NOT_RUNNING/ERROR.
The thing I'm wondering about is whether we should (a) adopt yet another exception to cover this case specifically (eg StreamsAlreadyClosedException or something), or (b) change the name of StreamsNotStarted to be a bit more generic, eg StreamsNotRunningException, and describe which state specifically in the error message of the exception, or (c) continue throwing StreamsNotStartedException only when the state is CREATED, and just continue to throw IllegalStateException if the state is one of the closed states I listed above.
Personally I think (c) makes the most sense here: then we don't need to update the KIP (other than to clarify we'll throw this for the other metadataForKey, etc methods in addition to #store). But mainly because IllegalStateException actually does seem appropriate for all state other than CREATED or RUNNING or REBALANCING -- all those other states are either terminal, or transition into a terminal state, so there's basically no way to recover and retry at this point. You'd need to bounce the app most likely. So from a user perspective you would want to catch and retry maybe on StreamsNotStartedException, but IllegalStateException maybe even should in fact kill the app so it can be restarted (eg k8s restarts the process/pod). I'm not sure we'd want to introduce another exception where the handling would be pretty much identical to not handling it at all. But I can be convinced, and maybe you have other ideas or opinions I haven't considered yet. Thoughts?
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-831536063
Ah, looks like that test just assumes no two UUIDs will have the same prefix. Fair assumption that no two UUIDs will be the _same_, but I guess that doesn't hold up if you cut off some of those digits. Anyways just following up on that, should be easy to fix
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-827302955
>It is a bit strange for me if use StreamsNotStartedException to replace IllegalStateException in the validateIsRunningOrRebalancing()
Isn't that what we're doing with the `KafkaStreams#store` method, though? It also invokes `validateIsRunningOrRebalancing()` and thus presumably will throw an IllegalStateException if a user invokes this on a KafkaStreams that hasn't been started. Maybe you can run the test you added with the actual changes commented out and see what gets thrown.
>I wonder if it worth to break the API compatibility? The trivial improvement will break the API compatibility(allMetadata(), allMetadataForStore(), queryMetadataForKey). It seems this change means that KIP-216 is no longer API Compatibility and the scope of the KIP will exceed Interactive Query related. WDYT?
Those methods are all, ultimately at least, related to Interactive Queries. My understanding is that they would be used to find out where to route a query for a specific key, for example. Plus they do take a particular store as a parameter, thus even `InvalidStateStore` makes about as much sense for these methods as it does for KafkaStreams#store.
Also just fyi, by sheer coincidence it seems this PR will end up landing in 3.0 which is a major version bump and thus (some) breaking changes are allowed. I think changing up exception handling in this way is acceptable (and as noted above it seems we are already doing so for the `#store` case, unless I'm missing something). Plus, it seems like compatibility is a bit nuanced in this particular case. Presumably a user will hit this case just because they forgot to call `start()` before invoking this API, therefore I would expect that any existing apps which are upgraded would already be sure to call start() first and it's only the new applications/new users which are likely to hit this particular exception.
That's my read of the situation, anyway. Thoughts?
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r621794196
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -344,7 +345,7 @@ private boolean isRunningOrRebalancing() {
private void validateIsRunningOrRebalancing() {
if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ throw new StreamsNotStartedException("KafkaStreams is not running. State is " + state + ".");
Review comment:
> So from a user perspective you would want to catch and retry maybe on StreamsNotStartedException, but IllegalStateException maybe even should in fact kill the app so it can be restarted (eg k8s restarts the process/pod).
Totally agree, sorry that I didn't point out this before. The user can just catch and retry when StreamsNotStartedException thrown. This is different from IllegalStateException. I don't think we need introduce another exception.
So I'm +1 on (c).
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng edited a comment on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng edited a comment on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-826589358
@ableegoldman Please take a look.
BTW, I encounter a [ClassDataAbstractionCoupling](https://checkstyle.org/config_metrics.html#ClassDataAbstractionCoupling) check style failure, so I update the **checkstyle/suppressions.xml** to avoid this failure. If there have a better way to solve this check style failure, please let me know.
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r620653928
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -1524,6 +1527,12 @@ public void cleanUp() {
* an InvalidStateStoreException is thrown upon store access.
*/
public <T> T store(final StoreQueryParameters<T> storeQueryParameters) {
+ synchronized (stateLock) {
+ if (state == State.CREATED) {
+ throw new StreamsNotStartedException("KafkaStreams is not started, you can retry and wait until to running.");
Review comment:
Technically we don't need to wait for it to be running, they just need to start it:
```suggestion
throw new StreamsNotStartedException("KafkaStreams has not been started, you can retry after calling start()");
```
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -1516,6 +1517,8 @@ public void cleanUp() {
*
* @param storeQueryParameters the parameters used to fetch a queryable store
* @return A facade wrapping the local {@link StateStore} instances
+ * @throws StreamsNotStartedException if Kafka Streams state is {@link KafkaStreams.State#CREATED CREATED}. Just
+ * retry and wait until to {@link KafkaStreams.State#RUNNING RUNNING}
Review comment:
```suggestion
* @throws StreamsNotStartedException If user has not started the KafkaStreams and it's still in {@link KafkaStreams.State#CREATED CREATED}.
```
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman merged pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman merged pull request #10597:
URL: https://github.com/apache/kafka/pull/10597
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-827296663
> LGTM, just some suggestions for the wording. Can we add a note to the section in the upgrade-guide that you added for the last exception?
Sure, will do. Thank for your reminder.
> By the way, I wonder if we should also throw this exception for the `allMetadata, `allMetadataForStore`, and `queryMetadataForKey`methods? It seems these methods along with`KafkaStreams#store`all do the same thing of calling`validateIsRunningOrRebalancing` which then checks on the KafkaStreams state and throws IllegalStateException if not in one of those two states. Imo all of these would benefit from the StreamsNotStartedException and it doesn't make sense to single that one out.
One thing may need remind, in the KIP-216, `StreamsNotStartedException` is sub class of `InvalidStateStoreException`. That means `StreamsNotStartedException` is an exception related to the StateStore. It is a bit strange for me if use `StreamsNotStartedException` to replace `IllegalStateException` in the `validateIsRunningOrRebalancing()`.
> Thoughts? I know it's not in the KIP but in this case it seems like a trivial improvement that would merit just a quick update on the KIP discussion thread but not a whole new KIP of its own
I wonder if it worth to break the API compatibility? The trivial improvement will break the API compatibility(`allMetadata()`, `allMetadataForStore()`, `queryMetadataForKey`). It seems this change means that KIP-216 is no longer API Compatibility and the scope of the KIP will exceed Interactive Query related. WDYT?
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-827475511
@ableegoldman Please take a look. :)
If all work in this PR is completed, I will update the KIP and discussion 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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-829564963
Thanks @vitojeng -- just to clarify, I was proposing that we do throw the StreamsNotStartedException for the `#allMetadata`, `#allMetadataForStore`, and `#queryMetadataForKey` methods in addition to just `#store`. Imo if we do this for `#store`, we should do it for all of these which have similar behavior and effect -- for example at the moment, all of these invoke `validateRunningOrRebalance` and thus would all throw IllegalStateException if the Streams has not been started. It seems odd to add this exception which perfectly addresses this specific case, and then only apply it to that one method.
What I was saying we should _not_ do was to throw StreamsNotStartedException for _any_ state that isn't RUNNING or REBALANCING. In other words we still only throw it for CREATED, but we throw the same exception across the board. Basically any method that currently invokes `validateRunningOrRebalancing` (which is `#store` + the ones listed above, I believe there are 5 methods in total)
Thoughts?
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r622464617
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -344,7 +345,7 @@ private boolean isRunningOrRebalancing() {
private void validateIsRunningOrRebalancing() {
if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ throw new StreamsNotStartedException("KafkaStreams is not running. State is " + state + ".");
Review comment:
SGTM
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-827317919
@ableegoldman
Got it, thank you for the explanation. :)
I am fine with this change. Let me apply StreamsNotStartedException to `validateIsRunningOrRebalancing()` in this PR, and update the KIP and discussion 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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r624155311
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -343,10 +344,16 @@ private boolean isRunningOrRebalancing() {
}
private void validateIsRunningOrRebalancing() {
- if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ synchronized (stateLock) {
+ if (state == State.CREATED) {
+ throw new StreamsNotStartedException("KafkaStreams is not started, you can retry and wait until to running.");
Review comment:
```suggestion
throw new StreamsNotStartedException("KafkaStreams has not been started, you can retry after calling start()");
```
I recommend avoiding using the term "running" here, since that has a specific meaning (ie, it implies you need to wait for the state to be RUNNING, which is not true here). We do tend to use the term running quite loosely in the code to sometimes refer to both running+rebalancing, but let's make this as clear as possible since it's user facing. It's bad enough that as devs you have to go to the method implementation to see if `isRunning` means literally just running, or if it also includes rebalancing state(s) 🙂
--
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:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-831524684
LGTM. Seems to be some unrelated known-flaky tests (RaftClusterTest and some Connect test), but also a unit test failure in RocksDBStore which I've never seen before. It's pretty suspicious since it's just a unit test and doesn't have any timeouts, setup, etc that might make it flaky. But it's hard to imagine that this PR could have caused that, and it did pass on the other two builds so I don't think it's related and will merge to trunk. But let's keep an eye on that ([KAFKA-12747](https://issues.apache.org/jira/browse/KAFKA-12747))
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r624229281
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -343,10 +344,16 @@ private boolean isRunningOrRebalancing() {
}
private void validateIsRunningOrRebalancing() {
- if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ synchronized (stateLock) {
+ if (state == State.CREATED) {
+ throw new StreamsNotStartedException("KafkaStreams is not started, you can retry and wait until to running.");
Review comment:
Agree. Thanks @ableegoldman !
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on a change in pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on a change in pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#discussion_r621901521
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -344,7 +345,7 @@ private boolean isRunningOrRebalancing() {
private void validateIsRunningOrRebalancing() {
if (!isRunningOrRebalancing()) {
- throw new IllegalStateException("KafkaStreams is not running. State is " + state + ".");
+ throw new StreamsNotStartedException("KafkaStreams is not running. State is " + state + ".");
Review comment:
> (c) continue throwing StreamsNotStartedException only when the state is CREATED, and just continue to throw IllegalStateException if the state is one of the closed states I listed above.
BTW, just want to remind that (c) only for `store()`.
Not include the public api which get store values (eg, `ReadOnlyKeyValueStore#get()`)
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-829215736
@ableegoldman Please take a look. :)
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng edited a comment on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng edited a comment on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-827317919
@ableegoldman
Got it, thank you for the explanation. :)
I am fine with this change. Let me apply `StreamsNotStartedException` to `validateIsRunningOrRebalancing()` in this PR, and update the KIP and discussion 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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-829640637
Oh, I get your point. This make sense and totally agree. Thanks @ableegoldman for explanation.
--
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:
users@infra.apache.org
[GitHub] [kafka] vitojeng commented on pull request #10597: KAFKA-5876: Apply StreamsNotStartedException for Interactive Queries
Posted by GitBox <gi...@apache.org>.
vitojeng commented on pull request #10597:
URL: https://github.com/apache/kafka/pull/10597#issuecomment-826589358
@ableegoldman Please take a look.
BTW, I encounter a [ClassDataAbstractionCoupling](https://checkstyle.org/config_metrics.html#ClassDataAbstractionCoupling) check style failure, so I update the **checkstyle/suppressions.xml** to avoid this failure. I am not sure that there have a better way to solve this failure.
--
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:
users@infra.apache.org