Matthias, Thanks for the review. I reply separately in the following sections.
--- Vito On Sun, Apr 8, 2018 at 1:30 PM, Matthias J. Sax <matth...@confluent.io> wrote: > Thanks for updating the KIP and sorry for the long pause... > > Seems you did a very thorough investigation of the code. It's useful to > understand what user facing interfaces are affected. (Some parts might be even too detailed for a KIP.) > I also think too detailed. Especially the section `Changes in call trace`. Do you think it should be removed? > > To summarize my current understanding of your KIP, the main change is to > introduce new exceptions that extend `InvalidStateStoreException`. > yep. Keep compatibility in this KIP is important things. I think the best way is that all new exceptions extend from `InvalidStateStoreException`. > > Some questions: > > - Why do we need ```? Could `InvalidStateStoreException` be used for > this purpose? > Does this question miss some word? > > - What the superclass of `StateStoreStreamThreadNotRunningException` > is? Should it be `InvalidStateStoreException` or `StateStoreFailException` > ? > > - Is `StateStoreClosed` a fatal or retryable exception ? > > I apologize for not well written parts. I tried to modify some code in the recent period and modify the KIP. The modification is now complete. Please look again. > > > -Matthias > > > On 2/21/18 5:15 PM, vito jeng wrote: > > Matthias, > > > > Sorry for not response these days. > > I just finished it. Please have a look. :) > > > > > > > > --- > > Vito > > > > On Wed, Feb 14, 2018 at 5:45 AM, Matthias J. Sax <matth...@confluent.io> > > wrote: > > > >> Is there any update on this KIP? > >> > >> -Matthias > >> > >> On 1/3/18 12:59 AM, vito jeng wrote: > >>> Matthias, > >>> > >>> Thank you for your response. > >>> > >>> I think you are right. We need to look at the state both of > >>> KafkaStreams and StreamThread. > >>> > >>> After further understanding of KafkaStreams thread and state store, > >>> I am currently rewriting the KIP. > >>> > >>> > >>> > >>> > >>> --- > >>> Vito > >>> > >>> On Fri, Dec 29, 2017 at 4:32 AM, Matthias J. Sax < > matth...@confluent.io> > >>> wrote: > >>> > >>>> Vito, > >>>> > >>>> Sorry for this late reply. > >>>> > >>>> There can be two cases: > >>>> > >>>> - either a store got migrated way and thus, is not hosted an the > >>>> application instance anymore, > >>>> - or, a store is hosted but the instance is in state rebalance > >>>> > >>>> For the first case, users need to rediscover the store. For the second > >>>> case, they need to wait until rebalance is finished. > >>>> > >>>> If KafkaStreams is in state ERROR, PENDING_SHUTDOWN, or NOT_RUNNING, > >>>> uses cannot query at all and thus they cannot rediscover or retry. > >>>> > >>>> Does this make sense? > >>>> > >>>> -Matthias > >>>> > >>>> On 12/20/17 12:54 AM, vito jeng wrote: > >>>>> Matthias, > >>>>> > >>>>> I try to clarify some concept. > >>>>> > >>>>> When streams state is REBALANCING, it means the user can just plain > >>>> retry. > >>>>> > >>>>> When streams state is ERROR or PENDING_SHUTDOWN or NOT_RUNNING, it > >> means > >>>>> state store migrated to another instance, the user needs to > rediscover > >>>> the > >>>>> store. > >>>>> > >>>>> Is my understanding correct? > >>>>> > >>>>> > >>>>> --- > >>>>> Vito > >>>>> > >>>>> On Sun, Nov 5, 2017 at 12:30 AM, Matthias J. Sax < > >> matth...@confluent.io> > >>>>> wrote: > >>>>> > >>>>>> Thanks for the KIP Vito! > >>>>>> > >>>>>> I agree with what Guozhang said. The original idea of the Jira was, > to > >>>>>> give different exceptions for different "recovery" strategies to the > >>>> user. > >>>>>> > >>>>>> For example, if a store is currently recreated, a user just need to > >> wait > >>>>>> and can query the store later. On the other hand, if a store go > >> migrated > >>>>>> to another instance, a user needs to rediscover the store instead > of a > >>>>>> "plain retry". > >>>>>> > >>>>>> Fatal errors might be a third category. > >>>>>> > >>>>>> Not sure if there is something else? > >>>>>> > >>>>>> Anyway, the KIP should contain a section that talks about this ideas > >> and > >>>>>> reasoning. > >>>>>> > >>>>>> > >>>>>> -Matthias > >>>>>> > >>>>>> > >>>>>> On 11/3/17 11:26 PM, Guozhang Wang wrote: > >>>>>>> Thanks for writing up the KIP. > >>>>>>> > >>>>>>> Vito, Matthias: one thing that I wanted to figure out first is what > >>>>>>> categories of errors we want to notify the users, if we only wants > to > >>>>>>> distinguish fatal v.s. retriable then probably we should rename the > >>>>>>> proposed StateStoreMigratedException / StateStoreClosedException > >>>> classes. > >>>>>>> And then from there we should list what are the possible internal > >>>>>>> exceptions ever thrown in those APIs in the call trace, and which > >>>>>>> exceptions should be wrapped to what others, and which ones should > be > >>>>>>> handled without re-throwing, and which ones should not be wrapped > at > >>>> all > >>>>>>> but directly thrown to user's face. > >>>>>>> > >>>>>>> Guozhang > >>>>>>> > >>>>>>> > >>>>>>> On Wed, Nov 1, 2017 at 11:09 PM, vito jeng <v...@is-land.com.tw> > >>>> wrote: > >>>>>>> > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> I'd like to start discuss KIP-216: > >>>>>>>> > >>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >>>>>>>> 216%3A+IQ+should+throw+different+exceptions+for+different+errors > >>>>>>>> > >>>>>>>> Please have a look. > >>>>>>>> Thanks! > >>>>>>>> > >>>>>>>> --- > >>>>>>>> Vito > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >>> > >> > >> > > > >
