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
