Thanks Matthias. The KIP is about InvalidStateStoreException. I pick `StateStoreNotAvailableException` because it may be more intuitive than `StreamsNotRunningException`.
No matter which one picked, it's good to me. --- Vito On Wed, Jan 22, 2020 at 7:44 AM Matthias J. Sax <matth...@confluent.io> wrote: > Thanks for updating the KIP! > > One last comment/question: you kept `StateStoreNotAvailableException` in > favor of `StreamsNotRunningException` (to merge both as suggested). > > I am wondering, if it might be better to keep > `StreamsNotRunningException` instead of > `StateStoreNotAvailableException`, because this exception is thrown if > Streams is in state PENDING_SHUTDOWN / NOT_RUNNING / ERROR ? > > > > -Matthias > > On 1/17/20 9:56 PM, John Roesler wrote: > > Thanks, Vito. I've just cast my vote. > > -John > > > > On Fri, Jan 17, 2020, at 21:32, Vito Jeng wrote: > >> Hi, folks, > >> > >> Just update the KIP, please take a look. > >> > >> Thanks! > >> > >> --- > >> Vito > >> > >> > >> On Fri, Jan 17, 2020 at 9:12 AM Vito Jeng <v...@is-land.com.tw> wrote: > >> > >>> Thanks Bill, John and Matthias. Glad you guys joined this discussion. > >>> I got a lot out of the discussion. > >>> > >>> I would like to update KIP-216 base on John's suggestion to remove the > >>> category. > >>> > >>> > >>> --- > >>> Vito > >>> > >>> > >>> On Fri, Jan 17, 2020 at 2:30 AM Matthias J. Sax <matth...@confluent.io > > > >>> wrote: > >>> > >>>>> Nevertheless, if we omit the categorization, it’s moot. > >>>> > >>>> Ack. > >>>> > >>>> I am fine to remove the middle tier. As John pointed out, it might be > >>>> weird to have only one concrete exception type per category. We can > also > >>>> explain in detail how to handle each exception in their JavaDocs. > >>>> > >>>> > >>>> -Matthias > >>>> > >>>> On 1/16/20 6:38 AM, Bill Bejeck wrote: > >>>>> Vito, > >>>>> > >>>>> Thanks for the updates, the KIP LGTM. > >>>>> > >>>>> -Bill > >>>>> > >>>>> On Wed, Jan 15, 2020 at 11:31 PM John Roesler <vvcep...@apache.org> > >>>> wrote: > >>>>> > >>>>>> Hi Vito, > >>>>>> > >>>>>> Haha, your archive game is on point! > >>>>>> > >>>>>> What Matthias said in that email is essentially what I figured was > the > >>>>>> rationale. It makes sense, but the point I was making is that this > >>>> really > >>>>>> doesn’t seem like a good way to structure a production app. On the > >>>> other > >>>>>> hand, considering the exception fatal has a good chance of avoiding > a > >>>>>> frustrating debug session if you just forgot to call start. > >>>>>> > >>>>>> Nevertheless, if we omit the categorization, it’s moot. > >>>>>> > >>>>>> It would be easy to add a categorization layer later if we want it, > but > >>>>>> not very easy to change it if we get it wrong. > >>>>>> > >>>>>> Thanks for your consideration! > >>>>>> -John > >>>>>> > >>>>>> On Wed, Jan 15, 2020, at 21:14, Vito Jeng wrote: > >>>>>>> Hi John, > >>>>>>> > >>>>>>> About `StreamsNotStartedException is strange` -- > >>>>>>> The original idea came from Matthias, two years ago. :) > >>>>>>> You can reference here: > >>>>>>> > >>>>>> > >>>> > https://mail-archives.apache.org/mod_mbox/kafka-dev/201806.mbox/%3c6c32083e-b63c-435b-521d-032d45cc5...@confluent.io%3e > >>>>>>> > >>>>>>> About omitting the categorization -- > >>>>>>> It looks reasonable. I'm fine with omitting the categorization but > not > >>>>>> very > >>>>>>> sure it is a good choice. > >>>>>>> Does any other folks provide opinion? > >>>>>>> > >>>>>>> > >>>>>>> Hi, folks, > >>>>>>> > >>>>>>> Just update the KIP-216, please take a look. > >>>>>>> > >>>>>>> --- > >>>>>>> Vito > >>>>>>> > >>>>>>> > >>>>>>> On Thu, Jan 16, 2020 at 6:35 AM Vito Jeng <v...@is-land.com.tw> > >>>> wrote: > >>>>>>> > >>>>>>>> > >>>>>>>> Hi, folks, > >>>>>>>> > >>>>>>>> Thank you suggestion, really appreciate it. :) > >>>>>>>> I understand your concern. I'll merge StreamsNotRunningException > and > >>>>>>>> StateStoreNotAvailableException. > >>>>>>>> > >>>>>>>> > >>>>>>>> --- > >>>>>>>> Vito > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Jan 16, 2020 at 6:22 AM John Roesler <vvcep...@apache.org > > > >>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hey Vito, > >>>>>>>>> > >>>>>>>>> Yes, thanks for the KIP. Sorry the discussion has been so long. > >>>>>>>>> Hopefully, we can close it out soon. > >>>>>>>>> > >>>>>>>>> I agree we can drop StreamsNotRunningException in favor of > >>>>>>>>> just StateStoreNotAvailableException. > >>>>>>>>> > >>>>>>>>> Unfortunately, I have some higher-level concerns. The value > >>>>>>>>> of these exceptions is that they tell you how to handle the > >>>>>>>>> various situations that can arise while querying a distributed > >>>>>>>>> data store. > >>>>>>>>> > >>>>>>>>> Ideally, as a caller, I should be able to just catch "retriable" > or > >>>>>>>>> "fatal" and handle them appropriately. Otherwise, there's no > >>>>>>>>> point in having categories, and we should just have all the > >>>>>>>>> exceptions extend InvalidStateStoreException. > >>>>>>>>> > >>>>>>>>> Presently, it's not possible to tell from just the > >>>>>>>>> "retriable"/"fatal" distinction what to do. You can tell > >>>>>>>>> from the descriptions of the various exceptions. E.g.: > >>>>>>>>> > >>>>>>>>> Retriable: > >>>>>>>>> * StreamsRebalancingException: the exact same call > >>>>>>>>> should just be retried until the rebalance is complete > >>>>>>>>> * StateStoreMigratedException: the store handle is > >>>>>>>>> now invalid, so you need to re-discover the instance > >>>>>>>>> and get a new handle on that instance. In other words, > >>>>>>>>> the query itself may be valid, but the particular method > >>>>>>>>> invocation on this particular instance has encountered > >>>>>>>>> a fatal exception. > >>>>>>>>> > >>>>>>>>> Fatal: > >>>>>>>>> * UnknownStateStoreException: this is truly fatal. No amount > >>>>>>>>> of retrying or re-discovering is going to get you a handle > on a > >>>>>>>>> store that doesn't exist in the cluster. > >>>>>>>>> * StateStoreNotAvailableException: this is actually recoverable, > >>>>>>>>> since the store might exist in the cluster, but isn't > available > >>>> on > >>>>>>>>> this particular instance (which is shut down or whatever). > >>>>>>>>> > >>>>>>>>> Personally, I'm not a fan of code bureaucracy, so I'm 100% fine > >>>>>>>>> with omitting the categorization and just having 5 subclasses > >>>>>>>>> of InvalidStateStoreException. Each of them would tell you > >>>>>>>>> how to handle them, and it's not too many to really > >>>>>>>>> understand and handle each one. > >>>>>>>>> > >>>>>>>>> If you really want to have a middle tier, I'd recommend: > >>>>>>>>> * RetryableStateStoreException: the exact same call > >>>>>>>>> should be repeated. > >>>>>>>>> * RecoverableStateStoreException: the store handle > >>>>>>>>> should be discarded and the caller should re-discover > >>>>>>>>> the location of the store and repeat the query on the > >>>>>>>>> correct instance. > >>>>>>>>> * FatalStateStoreException: the query/request is totally > >>>>>>>>> invalid and will never succeed. > >>>>>>>>> > >>>>>>>>> However, attempting to categorize the proposed exceptions > >>>>>>>>> reveals even problems with this categorization: > >>>>>>>>> Retriable: > >>>>>>>>> * StreamsRebalancingException > >>>>>>>>> Recoverable: > >>>>>>>>> * StateStoreMigratedException > >>>>>>>>> * StreamsNotRunningException > >>>>>>>>> Fatal: > >>>>>>>>> * UnknownStateStoreException > >>>>>>>>> > >>>>>>>>> But StreamsNotStartedException is strange... It means that > >>>>>>>>> one code path got a handle on a specific KafkaStreams object > >>>>>>>>> instance and sent it a query before another code path > >>>>>>>>> invoked the start() method on the exact same object instance. > >>>>>>>>> It seems like the most likely scenario is that whoever wrote > >>>>>>>>> the program just forgot to call start() before querying, in > >>>>>>>>> which case, retrying isn't going to help, and a fatal exception > >>>>>>>>> is more appropriate. I.e., it sounds like a "first 15 minutes > >>>>>>>>> experience" problem, and making it fatal would be more > >>>>>>>>> helpful. Even in a production context, there's no reason not > >>>>>>>>> to sequence your application startup such that you don't > >>>>>>>>> accept queries until after Streams is started. Thus, I guess > >>>>>>>>> I'd categorize it under "fatal". > >>>>>>>>> > >>>>>>>>> Regardless of whether you make it fatal or retriable, you'd > >>>>>>>>> still have a whole category with only one exception in it, > >>>>>>>>> and the other two categories only have two exceptions. > >>>>>>>>> Plus, as you pointed out in the KIP, you can't get all > >>>>>>>>> exceptions in all cases anyway: > >>>>>>>>> * store() can only throw NotStarted, NotRunning, > >>>>>>>>> and Unknown > >>>>>>>>> * actual store queries can only throw Rebalancing, > >>>>>>>>> Migrated, and NotRunning > >>>>>>>>> > >>>>>>>>> Thus, in practice also, there are exactly three categories > >>>>>>>>> and also exactly three exception types. It doesn't seem > >>>>>>>>> like there's a great advantage to the categories here. To > >>>>>>>>> avoid the categorization problem and also to clarify what > >>>>>>>>> exceptions can actually be thrown in different circumstances, > >>>>>>>>> it seems like we should just: > >>>>>>>>> * get rid of the middle tier and make all the exceptions > >>>>>>>>> extend InvalidStateStoreException > >>>>>>>>> * drop StateStoreNotAvailableException in favor of > >>>>>>>>> StreamsNotRunningException > >>>>>>>>> * clearly document on all public methods which exceptions > >>>>>>>>> need to be handled > >>>>>>>>> > >>>>>>>>> How do you feel about this? > >>>>>>>>> Thanks, > >>>>>>>>> -John > >>>>>>>>> > >>>>>>>>> On Wed, Jan 15, 2020, at 15:13, Bill Bejeck wrote: > >>>>>>>>>> Thanks for KIP Vito. > >>>>>>>>>> > >>>>>>>>>> Overall the KIP LGTM, but I'd have to agree with others on > merging > >>>>>> the > >>>>>>>>>> `StreamsNotRunningException` and > `StateStoreNotAvailableException` > >>>>>>>>> classes. > >>>>>>>>>> > >>>>>>>>>> Since in both cases, the thread state is in `PENDING_SHUTDOWN || > >>>>>>>>>> NOT_RUNNING || ERROR` I'm not even sure how we could distinguish > >>>>>> when to > >>>>>>>>>> use the different > >>>>>>>>>> exceptions. Maybe a good middle ground would be to have a > detailed > >>>>>>>>>> exception message. > >>>>>>>>>> > >>>>>>>>>> The KIP freeze is close, so I think if we can agree on this, we > can > >>>>>>>>> wrap up > >>>>>>>>>> the voting soon. > >>>>>>>>>> > >>>>>>>>>> Thanks, > >>>>>>>>>> Bill > >>>>>>>>>> > >>>>>>>>>> On Tue, Jan 14, 2020 at 2:12 PM Matthias J. Sax < > >>>>>> matth...@confluent.io> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Vito, > >>>>>>>>>>> > >>>>>>>>>>> It's still unclear to me what the advantage is, to have both > >>>>>>>>>>> `StreamsNotRunningException` and > >>>>>> `StateStoreNotAvailableException`? > >>>>>>>>>>> > >>>>>>>>>>> For both cased, the state is `PENDING_SHUTDOWN / NOT_RUNNING / > >>>>>> ERROR` > >>>>>>>>>>> and thus, for a user point of view, why does it matter if the > >>>>>> store is > >>>>>>>>>>> closed on not? I don't understand why/how this information > would > >>>>>> be > >>>>>>>>>>> useful? Do you have a concrete example in mind how a user would > >>>>>> react > >>>>>>>>>>> differently to both exceptions? > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> @Vinoth: about `StreamsRebalancingException` -- to me, it seems > >>>>>> best > >>>>>>>>> to > >>>>>>>>>>> actually do this on a per-query basis, ie, have an overload > >>>>>>>>>>> `KafkaStreams#store(...)` that takes a boolean flag that allow > to > >>>>>>>>>>> _disable_ the exception and opt-in to query a active store > during > >>>>>>>>>>> recovery. However, as KIP-535 actually introduces this change > in > >>>>>>>>>>> behavior, I think KIP-216 should not cover this, but KIP-535 > >>>>>> should be > >>>>>>>>>>> updated. I'll follow up on the other KIP thread to raise this > >>>>>> point. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> -Matthias > >>>>>>>>>>> > >>>>>>>>>>> On 1/11/20 12:26 AM, Vito Jeng wrote: > >>>>>>>>>>>> Hi, Matthias & Vinoth, > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks for the feedback. > >>>>>>>>>>>> > >>>>>>>>>>>>> What is still unclear to me is, what we gain by having both > >>>>>>>>>>>>> `StreamsNotRunningException` and > >>>>>>>>> `StateStoreNotAvailableException`. Both > >>>>>>>>>>>>> exception are thrown when KafkaStreams is in state > >>>>>>>>> PENDING_SHUTDOWN / > >>>>>>>>>>>>> NOT_RUNNING / ERROR. Hence, as a user what do I gain to know > >>>>>> if the > >>>>>>>>>>>>> state store is closed on not -- I can't query it anyway? > Maybe > >>>>>> I > >>>>>>>>> miss > >>>>>>>>>>>>> something thought? > >>>>>>>>>>>> > >>>>>>>>>>>> Yes, both `StreamsNotRunningException` and > >>>>>>>>>>>> `StateStoreNotAvailableException` are fatal exception. > >>>>>>>>>>>> But `StateStoreNotAvailableException` is fatal exception about > >>>>>> state > >>>>>>>>>>> store > >>>>>>>>>>>> related. > >>>>>>>>>>>> I think it would be helpful that if user need to distinguish > >>>>>> these > >>>>>>>>> two > >>>>>>>>>>>> different case to handle it. > >>>>>>>>>>>> > >>>>>>>>>>>> I'm not very sure, does that make sense? > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> --- > >>>>>>>>>>>> Vito > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On Fri, Jan 10, 2020 at 3:35 AM Vinoth Chandar < > >>>>>> vin...@apache.org> > >>>>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> +1 on merging `StreamsNotRunningException` and > >>>>>>>>>>>>> `StateStoreNotAvailableException`, both exceptions are fatal > >>>>>>>>> anyway. IMO > >>>>>>>>>>>>> its best to have these exceptions be about the state store > >>>>>> (and not > >>>>>>>>>>> streams > >>>>>>>>>>>>> state), to easier understanding. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Additionally, KIP-535 allows for querying of state stores in > >>>>>>>>> rebalancing > >>>>>>>>>>>>> state. So do we need the StreamsRebalancingException? > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On 2020/01/09 03:38:11, "Matthias J. Sax" < > >>>>>> matth...@confluent.io> > >>>>>>>>>>> wrote: > >>>>>>>>>>>>>> Sorry that I dropped the ball on this... > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks for updating the KIP. Overall LGTM now. Feel free to > >>>>>> start > >>>>>>>>> a > >>>>>>>>>>> VOTE > >>>>>>>>>>>>>> thread. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> What is still unclear to me is, what we gain by having both > >>>>>>>>>>>>>> `StreamsNotRunningException` and > >>>>>>>>> `StateStoreNotAvailableException`. > >>>>>>>>>>> Both > >>>>>>>>>>>>>> exception are thrown when KafkaStreams is in state > >>>>>>>>> PENDING_SHUTDOWN / > >>>>>>>>>>>>>> NOT_RUNNING / ERROR. Hence, as a user what do I gain to know > >>>>>> if > >>>>>>>>> the > >>>>>>>>>>>>>> state store is closed on not -- I can't query it anyway? > >>>>>> Maybe I > >>>>>>>>> miss > >>>>>>>>>>>>>> something thought? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On 11/3/19 6:07 PM, Vito Jeng wrote: > >>>>>>>>>>>>>>> Sorry for the late reply, thanks for the review. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> About `StateStoreMigratedException`: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Why is it only thrown if the state is REBALANCING? A store > >>>>>>>>> might be > >>>>>>>>>>>>>>>> migrated during a rebalance, and Kafka Streams might > resume > >>>>>>>>> back to > >>>>>>>>>>>>>>>> RUNNING state and afterward somebody tries to use an old > >>>>>> store > >>>>>>>>>>> handle. > >>>>>>>>>>>>>>>> Also, if state is REBALANCING, should we throw > >>>>>>>>>>>>>>>> `StreamThreadRebalancingException`? Hence, I think > >>>>>>>>>>>>>>>> `StateStoreMigratedException` does only make sense during > >>>>>>>>> `RUNNING` > >>>>>>>>>>>>> state. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thank you point this, already updated. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Why do we need to distinguish between > >>>>>>>>>>> `KafkaStreamsNotRunningException` > >>>>>>>>>>>>>>>> and `StateStoreNotAvailableException`? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> `KafkaStreamsNotRunningException` may be caused by various > >>>>>>>>> reasons, I > >>>>>>>>>>>>> think > >>>>>>>>>>>>>>> it would be helpful that the > >>>>>>>>>>>>>>> user can distinguish whether it is caused by the state > store > >>>>>>>>> closed. > >>>>>>>>>>>>>>> (Maybe I am wrong...) > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Last, why do we distinguish between `KafkaStreams` instance > >>>>>> and > >>>>>>>>>>>>>>>> `StreamsThread`? To me, it seems we should always refer to > >>>>>> the > >>>>>>>>>>>>> instance, > >>>>>>>>>>>>>>>> because that is the level of granularity in which we > >>>>>>>>> enable/disable > >>>>>>>>>>>>> IQ atm. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Totally agree. Do you mean the naming of state store > >>>>>> exceptions? > >>>>>>>>>>>>>>> I don't have special reason to distinguish these two. > >>>>>>>>>>>>>>> Your suggestion look more reasonable for the exception > >>>>>> naming. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Last, for `StateStoreMigratedException`, I would add that a > >>>>>> user > >>>>>>>>> need > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>> rediscover the store and cannot blindly retry as the store > >>>>>>>>> handle is > >>>>>>>>>>>>>>>> invalid and a new store handle must be retrieved. That is > a > >>>>>>>>>>> difference > >>>>>>>>>>>>>>>> to `StreamThreadRebalancingException` that allows for > >>>>>> "blind" > >>>>>>>>> retries > >>>>>>>>>>>>>>>> that either resolve (if the store is still on the same > >>>>>> instance > >>>>>>>>> after > >>>>>>>>>>>>>>>> rebalancing finishes, or changes to > >>>>>>>>> `StateStoreMigratedException` if > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>> store was migrated away during rebalancing). > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Nice, it's great! Thank you. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> The KIP already updated, please take a look. :) > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Wed, Oct 23, 2019 at 1:48 PM Matthias J. Sax < > >>>>>>>>>>> matth...@confluent.io > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Any update on this KIP? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On 10/7/19 3:35 PM, Matthias J. Sax wrote: > >>>>>>>>>>>>>>>>> Sorry for the late reply. The 2.4 deadline kept us quite > >>>>>> busy. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> About `StateStoreMigratedException`: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Why is it only thrown if the state is REBALANCING? A > store > >>>>>>>>> might be > >>>>>>>>>>>>>>>>> migrated during a rebalance, and Kafka Streams might > resume > >>>>>>>>> back to > >>>>>>>>>>>>>>>>> RUNNING state and afterward somebody tries to use an old > >>>>>> store > >>>>>>>>>>>>> handle. > >>>>>>>>>>>>>>>>> Also, if state is REBALANCING, should we throw > >>>>>>>>>>>>>>>>> `StreamThreadRebalancingException`? Hence, I think > >>>>>>>>>>>>>>>>> `StateStoreMigratedException` does only make sense during > >>>>>>>>> `RUNNING` > >>>>>>>>>>>>>>>> state. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Why do we need to distinguish between > >>>>>>>>>>>>> `KafkaStreamsNotRunningException` > >>>>>>>>>>>>>>>>> and `StateStoreNotAvailableException`? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Last, why do we distinguish between `KafkaStreams` > >>>>>> instance and > >>>>>>>>>>>>>>>>> `StreamsThread`? To me, it seems we should always refer > to > >>>>>> the > >>>>>>>>>>>>> instance, > >>>>>>>>>>>>>>>>> because that is the level of granularity in which we > >>>>>>>>> enable/disable > >>>>>>>>>>>>> IQ > >>>>>>>>>>>>>>>> atm. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Last, for `StateStoreMigratedException`, I would add > that a > >>>>>>>>> user > >>>>>>>>>>>>> need to > >>>>>>>>>>>>>>>>> rediscover the store and cannot blindly retry as the > store > >>>>>>>>> handle is > >>>>>>>>>>>>>>>>> invalid and a new store handle must be retrieved. That > is a > >>>>>>>>>>>>> difference > >>>>>>>>>>>>>>>>> to `StreamThreadRebalancingException` that allows for > >>>>>> "blind" > >>>>>>>>>>> retries > >>>>>>>>>>>>>>>>> that either resolve (if the store is still on the same > >>>>>> instance > >>>>>>>>>>> after > >>>>>>>>>>>>>>>>> rebalancing finishes, or changes to > >>>>>>>>> `StateStoreMigratedException` if > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>> store was migrated away during rebalancing). > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On 8/9/19 10:20 AM, Vito Jeng wrote: > >>>>>>>>>>>>>>>>>> My bad. The short link `https://shorturl.at/CDNT9` > <https://shorturl.at/CDNT9> > >>>> <https://shorturl.at/CDNT9> > >>>>>> <https://shorturl.at/CDNT9> > >>>>>>>>> <https://shorturl.at/CDNT9> > >>>>>>>>>>> <https://shorturl.at/CDNT9> > >>>>>>>>>>>>> <https://shorturl.at/CDNT9> > >>>>>>>>>>>>>>>> <https://shorturl.at/CDNT9> > >>>>>>>>>>>>>>>>>> <https://shorturl.at/CDNT9> seems incorrect. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Please use the following instead: > >>>>>> https://shorturl.at/bkKQU > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> --- > >>>>>>>>>>>>>>>>>> Vito > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On Fri, Aug 9, 2019 at 10:53 AM Vito Jeng < > >>>>>>>>> v...@is-land.com.tw> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Thanks, Matthias! > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> About `StreamThreadNotStartedException`: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Thank you for explanation. I agree with your opinion. > >>>>>>>>>>>>>>>>>>> `CompositeReadOnlyXxxStore#get()` would never throw > >>>>>>>>>>>>>>>>>>> `StreamThreadNotStartedException`. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> For the case that corresponding thread crashes after we > >>>>>>>>> handed out > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>> store handle. We may throw > >>>>>> `KafkaStreamsNotRunningException` > >>>>>>>>> or > >>>>>>>>>>>>>>>>>>> `StateStoreMigratedException`. > >>>>>>>>>>>>>>>>>>> In `StreamThreadStateStoreProvider`, we would throw > >>>>>>>>>>>>>>>>>>> `KafkaStreamsNotRunningException` when stream thread is > >>>>>> not > >>>>>>>>>>>>> running( > >>>>>>>>>>>>>>>>>>> https://shorturl.at/CDNT9) or throw > >>>>>>>>> `StateStoreMigratedException` > >>>>>>>>>>>>> when > >>>>>>>>>>>>>>>>>>> store is closed(https://shorturl.at/hrvAN). So I think > >>>>>> we > >>>>>>>>> do not > >>>>>>>>>>>>> need > >>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>> add a new type for this case. Does that make sense? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> About `KafkaStreamsNotRunningException` vs > >>>>>>>>>>>>>>>>>>> `StreamThreadNotRunningException`: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> I understand your point. I rename > >>>>>>>>>>>>> `StreamThreadNotRunningException` to > >>>>>>>>>>>>>>>>>>> `KafkaStreamsNotRunningException`. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> About check unknown state store names: > >>>>>>>>>>>>>>>>>>> Thank you for the hint. I add a new type > >>>>>>>>>>>>> `UnknownStateStoreException` > >>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>> this case. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Also, we should still have fatal exception > >>>>>>>>>>>>>>>>>>> `StateStoreNotAvailableException`? Not sure why you > >>>>>> remove > >>>>>>>>> it? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Thank you point this, already add it again. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> The KIP already updated, please take a look. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> --- > >>>>>>>>>>>>>>>>>>> Vito > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >> > >
