Good find Vito! What Navinder says makes sense -- as there is no RC for 2.5.0 yet, I took the liberty to do a HOTFIX PR so we can address the issue in 2.5.0 already.
https://github.com/apache/kafka/pull/8158 -Matthias On 2/21/20 11:11 PM, Navinder Brar wrote: > Hi Vito, > > I checked the code and I think you are right. If a user provides a wrong partition there will be NPE at tasks.get(keyTaskId).getStore(storeName) as that task is not available at this machine. > > I think we split the line: https://github.com/apache/kafka/blob/bbfecaef725456f648f03530d26a5395042966fa/streams/src/main/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProvider.java#L62 into 2 parts and check tasks.get(keyTaskId) separately. If it is null, we can throw an InvalidPartitionException. WDYS? > > Thanks, > Navinder > > > On Saturday, 22 February, 2020, 06:22:14 am IST, Vito Jeng <v...@is-land.com.tw> wrote: > > Hi, Matthias and Navinder, > > I have a question about the valid partition in > StreamThreadStateStoreProvider. > > In the StreamThreadStateStoreProvider#createKeyTaskId(storeName, partition): > https://github.com/apache/kafka/blob/bbfecaef725456f648f03530d26a5395042966fa/streams/src/main/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProvider.java#L103 > > We pass an integer as partition and then use this partition to create > TaskId instance in the topic group while loop. How do we make sure the > partition is valid? If we pass a correct storeName and a invalid partition > into createKeyTaskId() , it still looks can be created a new TaskId and > would not throw InvalidStateStorePartitionException. > > I guess this would cause a NullPointerException at line #62 because this > keyTaskId cannot found in the task list. > https://github.com/apache/kafka/blob/bbfecaef725456f648f03530d26a5395042966fa/streams/src/main/java/org/apache/kafka/streams/state/internals/StreamThreadStateStoreProvider.java#L62 > > Does this right? or there something wrong with me? > > --- > Vito > > > On Wed, Feb 5, 2020 at 2:53 AM Navinder Brar > <navinder_b...@yahoo.com.invalid> wrote: > >> Thanks Vito, for incorporating this. Makes sense. >> >> -Navinder >> >> >> On Wednesday, February 5, 2020, 12:17 AM, Matthias J. Sax < >> mj...@apache.org> wrote: >> > Thanks Vito! > > That makes sense to me. > > > On 2/1/20 11:29 PM, Vito Jeng wrote: >>>> Hi, folks, >>>> >>>> KIP-562(KAFKA-9445) already merged three days ago. >>>> >>>> I have updated KIP-216 to reflect the KIP-562. The main change is >>>> to introduce a new exception `InvalidStateStorePartitionException`, >>>> will be thrown when user requested partition not available. >>>> >>>> Please take a look and any feedback is welcome. Thanks Matthias for >>>> the reminder. >>>> >>>> --- Vito >>>> >>>> >>>> On Thu, Jan 23, 2020 at 2:13 PM Vito Jeng <v...@is-land.com.tw> >>>> wrote: >>>> >>>>> Got it, thanks Matthias. >>>>> >>>>> --- Vito >>>>> >>>>> >>>>> On Thu, Jan 23, 2020 at 1:31 PM Matthias J. Sax >>>>> <matth...@confluent.io> wrote: >>>>> >>>>>> Thanks Vito. >>>>>> >>>>>> I am also ok with either name. Just a personal slight >>>>>> preference, but not a important. >>>>>> >>>>>> >>>>>> -Matthias >>>>>> >>>>>> On 1/21/20 6:52 PM, Vito Jeng wrote: >>>>>>> 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/%3c6 > c32083e-b63c-435b-521d-032d45cc5...@confluent.io%3e > <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> >>>>>>>>>>>>>>>>>>>>>>>> <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 >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>> >> >> >> >> >>
signature.asc
Description: OpenPGP digital signature
