kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vito jeng <v...@is-land.com.tw>
Subject Re: [DISCUSS]KIP-216: IQ should throw different exceptions for different errors
Date Mon, 09 Apr 2018 22:33:36 GMT
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 <matthias@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 <matthias@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 <
> matthias@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 <
> >> matthias@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 <vito@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
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> >
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message