Thanks Navinder and Matthias. --- Vito
On Sat, Feb 22, 2020 at 8:12 PM Matthias J. Sax <mj...@apache.org> wrote: > 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> > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > > <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 > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>> > >> > >> > >> > >> > >> > >
