Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-22 Thread Vito Jeng
Thanks Navinder and Matthias. --- Vito On Sat, Feb 22, 2020 at 8:12 PM Matthias J. Sax 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/a

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-22 Thread Matthias J. Sax
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-21 Thread Navinder Brar
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/bbfecaef725456f648f03530d26a5

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-21 Thread Vito Jeng
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/kaf

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-04 Thread Navinder Brar
Thanks Vito, for incorporating this. Makes sense. -Navinder On Wednesday, February 5, 2020, 12:17 AM, Matthias J. Sax wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Thanks Vito! That makes sense to me. On 2/1/20 11:29 PM, Vito Jeng wrote: > Hi, folks, > > KIP-562(KAFKA-9445) alrea

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-04 Thread Matthias J. Sax
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 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 `Inv

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-02-01 Thread Vito Jeng
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 w

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-22 Thread Vito Jeng
Got it, thanks Matthias. --- Vito On Thu, Jan 23, 2020 at 1:31 PM Matthias J. Sax 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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-22 Thread Matthias J. Sax
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 > t

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-21 Thread Vito Jeng
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 wrote: > Thanks for

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-21 Thread Matthias J. Sax
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`, becaus

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-17 Thread John Roesler
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 wrote: > > > Thanks Bill, John and Matthias. Glad you guys joined

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-17 Thread Vito Jeng
Hi, folks, Just update the KIP, please take a look. Thanks! --- Vito On Fri, Jan 17, 2020 at 9:12 AM Vito Jeng 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 r

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-16 Thread John Roesler
Thanks for your patience, Vito! -John On Thu, Jan 16, 2020, at 19:12, Vito Jeng 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. > > > --- >

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-16 Thread Vito Jeng
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 wrote: > > Nevertheless, if we omit the categoriz

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-16 Thread Matthias J. Sax
> 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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-16 Thread Bill Bejeck
Vito, Thanks for the updates, the KIP LGTM. -Bill On Wed, Jan 15, 2020 at 11:31 PM John Roesler 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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-15 Thread John Roesler
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 exceptio

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-15 Thread Vito Jeng
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/%3c6c32083e-b63c-435b-521d-032d45cc5...@confluent.io%3e About omitting the categorization -- It

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-15 Thread Vito Jeng
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 wrote: > Hey Vito, > > Yes, thanks for the KIP. Sorry the discussion has been

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-15 Thread John Roesler
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 th

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-15 Thread Bill Bejeck
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-14 Thread Matthias J. Sax
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'

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-11 Thread Vito Jeng
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-10 Thread Vinoth Chandar
+1 on a streams config to control this behavior. I have no strong opinions on the default, but I would pick allowing to query if standbys are enabled else throw the exception.. But we can keep it simpler, throw exception by default and have a flag to turn it off, as you suggest as well. On Thu, Ja

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-09 Thread Matthias J. Sax
Good question about `StreamsRebalancingException` -- when this KIP was started, KIP-535 was not on the horizon yet. What I am wondering is, if we should allow people to opt-in into querying during a rebalance, or to be more precise during a restore (if a state store is not migrated, it will be up-

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-09 Thread Vinoth Chandar
+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 reba

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2020-01-08 Thread Matthias J. Sax
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-11-03 Thread Vito Jeng
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 ha

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-10-22 Thread Matthias J. Sax
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 mi

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-10-07 Thread Matthias J. Sax
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 stor

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-08-09 Thread Vito Jeng
My bad. The short link `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 wrote: > Thanks, Matthias! > > > About `StreamThreadNotStartedException`: > > Thank y

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-08-08 Thread Vito Jeng
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 th

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-07-29 Thread Matthias J. Sax
Any update on this KIP Vito? On 7/11/19 4:26 PM, Matthias J. Sax wrote: > Thanks Vito! I think the KIP shapes out nicely! > > > To answer the open question you raised (I also adjust my answers based > on the latest KIP update) > > > > About `StreamThreadNotStartedException`: I understand wha

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-07-11 Thread Matthias J. Sax
Thanks Vito! I think the KIP shapes out nicely! To answer the open question you raised (I also adjust my answers based on the latest KIP update) About `StreamThreadNotStartedException`: I understand what you pointed out. However, I think we can consider the following: If a thread is not starte

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-07-05 Thread Vito Jeng
Hi, Mattias, Just completed the modification of KIP, please take a look when you are available. --- Vito On Wed, Jul 3, 2019 at 9:07 PM Vito Jeng wrote: > Hi, Matthias, > > This is second part. > > > For the internal exceptions: > > > > `StateStoreClosedException` -- why can it be wrapped as

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-07-03 Thread Vito Jeng
Hi, Matthias, This is second part. > For the internal exceptions: > > `StateStoreClosedException` -- why can it be wrapped as > `StreamThreadNotStartedException` ? It seems that the later would only > be thrown by `KafkaStreams#store()` and thus would be throw directly. Both `StateStoreClosedExc

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-06-22 Thread Vito Jeng
Hi, Mattias, Sorry for late reply. Nice to hear from you, thank you for re-reading the discussion thread and help to finish the KIP. I spend some time rereading our past discussions and updating the KIP. Hope the following reply can have more clarification. > For example, StreamThreadNotStartedEx

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2019-06-05 Thread Matthias J. Sax
Hi Vito, sorry for dropping this discussion on the floor a while back. I was just re-reading the KIP and discussion thread, and I think it is shaping out nicely! I like the overall hierarchy of the exception classes. Some things are still not 100% clear: You listed all methods that may throw a

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-11-11 Thread Vito Jeng
Hi, Matthias, KIP already updated. > - StateStoreClosedException: > will be wrapped to StateStoreMigratedException or StateStoreNotAvailableException later. > Can you clarify the cases (ie, when will it be wrapped with the one or the other)? For example, in the implementation(CompositeReadOnly

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-11-11 Thread Vito Jeng
Hi, Matthias, Sorry for the late reply. > I am wondering what the semantic impact/change is, if we introduce > `RetryableStateStoreException` and `FatalStateStoreException` that both > inherit from it. While I like the introduction of both from a high level > point of view, I just want to make su

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-11-06 Thread Matthias J. Sax
Hey Vito, I saw that you updated your PR, but did not reply to my last comments. Any thoughts? -Matthias On 10/19/18 10:34 AM, Matthias J. Sax wrote: > Glad to have you back Vito :) > > Some follow up thoughts: > > - the current `InvalidStateStoreException` is documents as being > sometimes

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-19 Thread Matthias J. Sax
Glad to have you back Vito :) Some follow up thoughts: - the current `InvalidStateStoreException` is documents as being sometimes retry-able. From the JavaDocs: > These exceptions may be transient [...] Hence, it is valid to backoff and > retry when handling this exception. I am wondering wha

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-17 Thread vito jeng
Just open a PR for further discussion: https://github.com/apache/kafka/pull/5814 Any suggestion is welcome. Thanks! --- Vito On Thu, Oct 11, 2018 at 12:14 AM vito jeng wrote: > Hi John, > > Thanks for reviewing the KIP. > > > I didn't follow the addition of a new method to the QueryableStoreT

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-10 Thread vito jeng
Hi John, Thanks for reviewing the KIP. > 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? To support the new exception types, I would check stream state in the following classes: -

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-08 Thread John Roesler
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"

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-05 Thread Guozhang Wang
Thanks for the explanation, that makes sense. Guozhang On Mon, Jun 25, 2018 at 2:28 PM, Matthias J. Sax 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 thr

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-10-05 Thread vito jeng
Matthias, > Sorry to hear! Get well soon! I'm back! :) Base on many suggestion and discussion, I already rewrite the KIP. Please take a look: https://cwiki.apache.org/confluence/display/KAFKA/KIP-216%3A+IQ+should+throw+different+exceptions+for+different+errors > I see. If this is the intention

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-06-25 Thread Matthias J. Sax
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-06-25 Thread Guozhang Wang
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 P

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-06-24 Thread Matthias J. Sax
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. >> >> StreamThrea

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-06-22 Thread vito jeng
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-06-10 Thread Matthias J. Sax
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 `Stat

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-24 Thread vito jeng
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 wrote: > Hi Vito, > > Thanks for the KIP, overall it's a +1 from me. > > At this point, the only thing I woul

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-24 Thread vito jeng
Hi, John, Thanks for the comment! KIP typically discuss the public interfaces, non-user-facing detail is not necessary. I think your opinion is correct. But in this KIP, when a state store exception throw, the user will see the full exception stacktrace in the log, including the internal excepti

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-24 Thread vito jeng
Hi, Matthias, Thanks for the review! I have updated the KIP, please look again. Thanks. --- Vito On Thu, Apr 19, 2018 at 9:58 PM, Matthias J. Sax wrote: > Thanks for clarification! That makes sense to me. > > Can you update the KIP to make those suggestions explicit? > > > -Matthias > > On

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-20 Thread Bill Bejeck
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,

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-20 Thread Guozhang Wang
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 dif

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-20 Thread John Roesler
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 s

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-19 Thread Matthias J. Sax
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 (incomple

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-18 Thread vito jeng
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 th

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-16 Thread Matthias J. Sax
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-09 Thread vito jeng
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 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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-04-07 Thread Matthias J. Sax
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.) To summarize my current understanding of your KIP, the main c

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-02-21 Thread vito jeng
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 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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-02-13 Thread Matthias J. Sax
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,

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2018-01-03 Thread vito jeng
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-28 Thread Matthias J. Sax
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, t

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-20 Thread vito jeng
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 corr

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-06 Thread vito jeng
Thank you for your attention to this KIP and assistance. --- Vito On Wed, Dec 6, 2017 at 2:35 PM, Matthias J. Sax wrote: > Thanks for the info. There is no hurry. Was just curious :) > > -Matthias > > > On 12/5/17 7:34 PM, vito jeng wrote: > > Matthias, > > > > Still in progress. I've been bu

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-05 Thread Matthias J. Sax
Thanks for the info. There is no hurry. Was just curious :) -Matthias On 12/5/17 7:34 PM, vito jeng wrote: > Matthias, > > Still in progress. I've been busy in recent weeks with my job. > > But I believe I will update this KIP within few days. > > > > > --- > Vito > > On Tue, Dec 5, 2017

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-05 Thread vito jeng
Matthias, Still in progress. I've been busy in recent weeks with my job. But I believe I will update this KIP within few days. --- Vito On Tue, Dec 5, 2017 at 8:05 AM, Matthias J. Sax wrote: > Vito, > > is there any update with regard to this KIP? > > > -Matthias > > On 11/5/17 6:11 PM, vi

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-12-04 Thread Matthias J. Sax
Vito, is there any update with regard to this KIP? -Matthias On 11/5/17 6:11 PM, vito jeng wrote: > Thanks, Guozhang and Matthias. Your comments very useful for me. > > I'll update KIP and keep going on. > > > > --- > Vito > > On Sun, Nov 5, 2017 at 12:30 AM, Matthias J. Sax > wrote: > >

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-11-05 Thread vito jeng
Thanks, Guozhang and Matthias. Your comments very useful for me. I'll update KIP and keep going on. --- Vito On Sun, Nov 5, 2017 at 12:30 AM, Matthias J. Sax wrote: > Thanks for the KIP Vito! > > I agree with what Guozhang said. The original idea of the Jira was, to > give different exceptio

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-11-04 Thread Matthias J. Sax
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

Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors

2017-11-03 Thread Guozhang Wang
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 / StateStoreClosedExcept