Hi Vito, I'm glad to hear you're well again!
I didn't follow the addition of a new method to the QueryableStoreType interface. Can you elaborate why this is necessary to support the new exception types? Also, looking over your KIP again, it seems valuable to introduce "retriable store exception" and "fatal store exception" marker interfaces that the various exceptions can mix in. It would be nice from a usability perspective to be able to just log and retry on any "retriable" exception and log and shutdown on any fatal exception. Thanks, -John On Fri, Oct 5, 2018 at 11:47 AM Guozhang Wang <wangg...@gmail.com> wrote: > Thanks for the explanation, that makes sense. > > > Guozhang > > > On Mon, Jun 25, 2018 at 2:28 PM, Matthias J. Sax <matth...@confluent.io> > wrote: > > > The scenario I had I mind was, that KS is started in one thread while a > > second thread has a reference to the object to issue queries. > > > > If a query is issue before the "main thread" started KS, and the "query > > thread" knows that it will eventually get started, it can retry. On the > > other hand, if KS is in state PENDING_SHUTDOWN or DEAD, it is impossible > > to issue any query against it now or in the future and thus the error is > > not retryable. > > > > > > -Matthias > > > > On 6/25/18 10:15 AM, Guozhang Wang wrote: > > > I'm wondering if StreamThreadNotStarted could be merged into > > > StreamThreadNotRunning, because I think users' handling logic for the > > third > > > case would be likely the same as the second. Do you have some scenarios > > > where users may want to handle them differently? > > > > > > Guozhang > > > > > > On Sun, Jun 24, 2018 at 5:25 PM, Matthias J. Sax < > matth...@confluent.io> > > > wrote: > > > > > >> Sorry to hear! Get well soon! > > >> > > >> It's not a big deal if the KIP stalls a little bit. Feel free to pick > it > > >> up again when you find time. > > >> > > >>>>> Is `StreamThreadNotRunningException` really an retryable error? > > >>>> > > >>>> When KafkaStream state is REBALANCING, I think it is a retryable > > error. > > >>>> > > >>>> StreamThreadStateStoreProvider#stores() will throw > > >>>> StreamThreadNotRunningException when StreamThread state is not > > >> RUNNING. The > > >>>> user can retry until KafkaStream state is RUNNING. > > >> > > >> I see. If this is the intention, than I would suggest to have two (or > > >> maybe three) different exceptions: > > >> > > >> - StreamThreadRebalancingException (retryable) > > >> - StreamThreadNotRunning (not retryable -- thrown if in state > > >> PENDING_SHUTDOWN or DEAD > > >> - maybe StreamThreadNotStarted (for state CREATED) > > >> > > >> The last one is tricky and could also be merged into one of the first > > >> two, depending if you want to argue that it's retryable or not. (Just > > >> food for though -- not sure what others think.) > > >> > > >> > > >> > > >> -Matthias > > >> > > >> On 6/22/18 8:06 AM, vito jeng wrote: > > >>> Matthias, > > >>> > > >>> Thank you for your assistance. > > >>> > > >>>> what is the status of this KIP? > > >>> > > >>> Unfortunately, there is no further progress. > > >>> About seven weeks ago, I was injured in sports. I had a broken wrist > on > > >>> my left wrist. > > >>> Many jobs are affected, including this KIP and implementation. > > >>> > > >>> > > >>>> I just re-read it, and have a couple of follow up comments. Why do > we > > >>>> discuss the internal exceptions you want to add? Also, do we really > > need > > >>>> them? Can't we just throw the correct exception directly instead of > > >>>> wrapping it later? > > >>> > > >>> I think you may be right. As I say in the previous: > > >>> "The original idea is that we can distinguish different state store > > >>> exception for different handling. But to be honest, I am not quite > sure > > >>> this is necessary. Maybe have some change during implementation." > > >>> > > >>> During the implementation, I also feel we maybe not need wrapper it. > > >>> We can just throw the correctly directly. > > >>> > > >>> > > >>>> Is `StreamThreadNotRunningException` really an retryable error? > > >>> > > >>> When KafkaStream state is REBALANCING, I think it is a retryable > error. > > >>> > > >>> StreamThreadStateStoreProvider#stores() will throw > > >>> StreamThreadNotRunningException when StreamThread state is not > > RUNNING. > > >> The > > >>> user can retry until KafkaStream state is RUNNING. > > >>> > > >>> > > >>>> When would we throw an `StateStoreEmptyException`? The semantics is > > >>> unclear to me atm. > > >>> > > >>>> When the state is RUNNING, is `StateStoreClosedException` a > retryable > > >>> error? > > >>> > > >>> These two comments will be answered in another mail. > > >>> > > >>> > > >>> > > >>> --- > > >>> Vito > > >>> > > >>> On Mon, Jun 11, 2018 at 8:12 AM, Matthias J. Sax < > > matth...@confluent.io> > > >>> wrote: > > >>> > > >>>> Vito, > > >>>> > > >>>> what is the status of this KIP? > > >>>> > > >>>> I just re-read it, and have a couple of follow up comments. Why do > we > > >>>> discuss the internal exceptions you want to add? Also, do we really > > need > > >>>> them? Can't we just throw the correct exception directly instead of > > >>>> wrapping it later? > > >>>> > > >>>> When would we throw an `StateStoreEmptyException`? The semantics is > > >>>> unclear to me atm. > > >>>> > > >>>> Is `StreamThreadNotRunningException` really an retryable error? > > >>>> > > >>>> When the state is RUNNING, is `StateStoreClosedException` a > retryable > > >>>> error? > > >>>> > > >>>> One more nits: ReadOnlyWindowStore got a new method #fetch(K key, > long > > >>>> time); that should be added > > >>>> > > >>>> > > >>>> Overall I like the KIP but some details are still unclear. Maybe it > > >>>> might help if you open an PR in parallel? > > >>>> > > >>>> > > >>>> -Matthias > > >>>> > > >>>> On 4/24/18 8:18 AM, vito jeng wrote: > > >>>>> Hi, Guozhang, > > >>>>> > > >>>>> Thanks for the comment! > > >>>>> > > >>>>> > > >>>>> > > >>>>> Hi, Bill, > > >>>>> > > >>>>> I'll try to make some update to make the KIP better. > > >>>>> > > >>>>> Thanks for the comment! > > >>>>> > > >>>>> > > >>>>> --- > > >>>>> Vito > > >>>>> > > >>>>> On Sat, Apr 21, 2018 at 5:40 AM, Bill Bejeck <bbej...@gmail.com> > > >> wrote: > > >>>>> > > >>>>>> Hi Vito, > > >>>>>> > > >>>>>> Thanks for the KIP, overall it's a +1 from me. > > >>>>>> > > >>>>>> At this point, the only thing I would change is possibly removing > > the > > >>>>>> listing of all methods called by the user and the listing of all > > store > > >>>>>> types and focus on what states result in which exceptions thrown > to > > >> the > > >>>>>> user. > > >>>>>> > > >>>>>> Thanks, > > >>>>>> Bill > > >>>>>> > > >>>>>> On Fri, Apr 20, 2018 at 2:10 PM, Guozhang Wang < > wangg...@gmail.com> > > >>>> wrote: > > >>>>>> > > >>>>>>> Thanks for the KIP Vito! > > >>>>>>> > > >>>>>>> I made a pass over the wiki and it looks great to me. I'm +1 on > the > > >>>> KIP. > > >>>>>>> > > >>>>>>> About the base class InvalidStateStoreException itself, I'd > > actually > > >>>>>>> suggest we do not deprecate it but still expose it as part of the > > >>>> public > > >>>>>>> API, for people who do not want to handle these cases differently > > (if > > >>>> we > > >>>>>>> deprecate it then we are enforcing them to capture all three > > >> exceptions > > >>>>>>> one-by-one). > > >>>>>>> > > >>>>>>> > > >>>>>>> Guozhang > > >>>>>>> > > >>>>>>> > > >>>>>>> On Fri, Apr 20, 2018 at 9:14 AM, John Roesler <j...@confluent.io > > > > >>>> wrote: > > >>>>>>> > > >>>>>>>> Hi Vito, > > >>>>>>>> > > >>>>>>>> Thanks for the KIP! > > >>>>>>>> > > >>>>>>>> I think it's much nicer to give callers different exceptions to > > tell > > >>>>>> them > > >>>>>>>> whether the state store got migrated, whether it's still > > >> initializing, > > >>>>>> or > > >>>>>>>> whether there's some unrecoverable error. > > >>>>>>>> > > >>>>>>>> In the KIP, it's typically not necessary to discuss > > non-user-facing > > >>>>>>> details > > >>>>>>>> such as what exceptions we will throw internally. The KIP is > > >> primarily > > >>>>>> to > > >>>>>>>> discuss public interface changes. > > >>>>>>>> > > >>>>>>>> You might consider simply removing all the internal details from > > the > > >>>>>> KIP, > > >>>>>>>> which will have the dual advantage that it makes the KIP smaller > > and > > >>>>>>> easier > > >>>>>>>> to agree on, as well as giving you more freedom in the internal > > >>>> details > > >>>>>>>> when it comes to implementation. > > >>>>>>>> > > >>>>>>>> I like your decision to have your refined exceptions extend > > >>>>>>>> InvalidStateStoreException to ensure backward compatibility. > Since > > >> we > > >>>>>>> want > > >>>>>>>> to encourage callers to catch the more specific exceptions, and > we > > >>>>>> don't > > >>>>>>>> expect to ever throw a raw InvalidStateStoreException anymore, > you > > >>>>>> might > > >>>>>>>> consider adding the @Deprecated annotation to > > >>>>>> InvalidStateStoreException. > > >>>>>>>> This will gently encourage callers to migrate to the new > exception > > >> and > > >>>>>>> open > > >>>>>>>> the possibility of removing InvalidStateStoreException entirely > > in a > > >>>>>>> future > > >>>>>>>> release. > > >>>>>>>> > > >>>>>>>> Thanks, > > >>>>>>>> -John > > >>>>>>>> > > >>>>>>>> On Thu, Apr 19, 2018 at 8:58 AM, Matthias J. Sax < > > >>>>>> matth...@confluent.io> > > >>>>>>>> wrote: > > >>>>>>>> > > >>>>>>>>> Thanks for clarification! That makes sense to me. > > >>>>>>>>> > > >>>>>>>>> Can you update the KIP to make those suggestions explicit? > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> -Matthias > > >>>>>>>>> > > >>>>>>>>> On 4/18/18 2:19 PM, vito jeng wrote: > > >>>>>>>>>> Matthias, > > >>>>>>>>>> > > >>>>>>>>>> Thanks for the feedback! > > >>>>>>>>>> > > >>>>>>>>>>> It's up to you to keep the details part in the KIP or not. > > >>>>>>>>>> > > >>>>>>>>>> Got it! > > >>>>>>>>>> > > >>>>>>>>>>> The (incomplete) question was, if we need > > >>>>>> `StateStoreFailException` > > >>>>>>> or > > >>>>>>>>>>> if existing `InvalidStateStoreException` could be used? Do > you > > >>>>>>> suggest > > >>>>>>>>>>> that `InvalidStateStoreException` is not thrown at all > anymore, > > >>>>>> but > > >>>>>>>> only > > >>>>>>>>>>> the new sub-classes (just to get a better understanding). > > >>>>>>>>>> > > >>>>>>>>>> Yes. I suggest that `InvalidStateStoreException` is not thrown > > at > > >>>>>> all > > >>>>>>>>>> anymore, > > >>>>>>>>>> but only new sub-classes. > > >>>>>>>>>> > > >>>>>>>>>>> Not sure what this sentence means: > > >>>>>>>>>>>> The internal exception will be wrapped as category exception > > >>>>>>> finally. > > >>>>>>>>>>> Can you elaborate? > > >>>>>>>>>> > > >>>>>>>>>> For example, `StreamThreadStateStoreProvider#stores()` will > > throw > > >>>>>>>>>> `StreamThreadNotRunningException`(internal exception). > > >>>>>>>>>> And then the internal exception will be wrapped as > > >>>>>>>>>> `StateStoreRetryableException` or `StateStoreFailException` > > during > > >>>>>>> the > > >>>>>>>>>> `KafkaStreams.store()` and throw to the user. > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>>> Can you explain the purpose of the "internal exceptions". > It's > > >>>>>>> unclear > > >>>>>>>>>> to me atm why they are introduced. > > >>>>>>>>>> > > >>>>>>>>>> Hmmm...the purpose of the "internal exceptions" is to > > distinguish > > >>>>>>>> between > > >>>>>>>>>> the different kinds of InvalidStateStoreException. > > >>>>>>>>>> The original idea is that we can distinguish different state > > store > > >>>>>>>>>> exception for > > >>>>>>>>>> different handling. > > >>>>>>>>>> But to be honest, I am not quite sure this is necessary. Maybe > > >> have > > >>>>>>>>>> some change during implementation. > > >>>>>>>>>> > > >>>>>>>>>> Does it make sense? > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>>> --- > > >>>>>>>>>> Vito > > >>>>>>>>>> > > >>>>>>>>>> On Mon, Apr 16, 2018 at 5:59 PM, Matthias J. Sax < > > >>>>>>>> matth...@confluent.io> > > >>>>>>>>>> wrote: > > >>>>>>>>>> > > >>>>>>>>>>> Thanks for the update Vito! > > >>>>>>>>>>> > > >>>>>>>>>>> It's up to you to keep the details part in the KIP or not. > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> The (incomplete) question was, if we need > > >>>>>> `StateStoreFailException` > > >>>>>>> or > > >>>>>>>>>>> if existing `InvalidStateStoreException` could be used? Do > you > > >>>>>>> suggest > > >>>>>>>>>>> that `InvalidStateStoreException` is not thrown at all > anymore, > > >>>>>> but > > >>>>>>>> only > > >>>>>>>>>>> the new sub-classes (just to get a better understanding). > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> Not sure what this sentence means: > > >>>>>>>>>>> > > >>>>>>>>>>>> The internal exception will be wrapped as category exception > > >>>>>>> finally. > > >>>>>>>>>>> > > >>>>>>>>>>> Can you elaborate? > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> Can you explain the purpose of the "internal exceptions". > It's > > >>>>>>> unclear > > >>>>>>>>>>> to me atm why they are introduced. > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> -Matthias > > >>>>>>>>>>> > > >>>>>>>>>>> On 4/10/18 12:33 AM, vito jeng wrote: > > >>>>>>>>>>>> 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 `StateStoreStreamThreadNotRunni > > >>>>>>>> ngException` > > >>>>>>>>>>>>> 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 > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> -- > > >>>>>>> -- Guozhang > > >>>>>>> > > >>>>>> > > >>>>> > > >>>> > > >>>> > > >>> > > >> > > >> > > > > > > > > > > > > > -- > -- Guozhang >
