Matt,
I agree with Matthias on not to altering the serializer as it's used by
multiple components.
Matthias,
- the proposed method accepts a `ProducerRecord` -- it might be good to
explain why this cannot be done in a type safe way (ie, missing generics)
To accept different types of records from multiple topologies, I have to
define the ProducerRecord without generics.
- `AlwaysProductionExceptionHandler` ->
`AlwaysContinueProductionExceptionHandler`
Updated the typo error in KIP.
- `DefaultProductionExceptionHandler` is not mentioned
The `handleSerializationException` method in the
`ProductionExceptionHandler` interface will have default implementation
that is set to FAIL by default.
This is done to avoid any changes in the user implementation. So, I didn't
mentioned the `DefaultProductionExceptionHandler` class. Updated the KIP.
- Why do you distinguish between `ClassCastException` and "any other
unchecked exception? Both second case seems to include the first one?
In SinkNode.java#93
<https://github.com/apache/kafka/blob/87cc31c4e7ea36e7e832a1d02d71480a91a75293/streams/src/main/java/org/apache/kafka/streams/processor/internals/SinkNode.java#L93>
on
hitting `ClassCastException`, we are halting the streams as it's a fatal
error.
To keep the original behavior, I've to distinguish the exceptions.
On Thu, Dec 6, 2018 at 10:44 PM Matthias J. Sax <matth...@confluent.io>
wrote:
> Well, that's exactly the point. The serializer should not be altered
> IMHO because this would have impact on other components. Also, for
> applications that use KafkaProducer directly, they can catch any
> serialization exception and react to it. Hence, I don't don't see a
> reason to change the serializer interface.
>
> Instead, it seems better to solve this issue in Streams by allowing to
> skip over a record for this case.
>
> Some more comments on the KIP:
>
> - the proposed method accepts a `ProducerRecord` -- it might be good to
> explain why this cannot be done in a type safe way (ie, missing generics)
>
> - `AlwaysProductionExceptionHandler` ->
> `AlwaysContinueProductionExceptionHandler`
>
> - `DefaultProductionExceptionHandler` is not mentioned
>
> - Why do you distinguish between `ClassCastException` and "any other
> unchecked exception? Both second case seems to include the first one?
>
>
>
> -Matthias
>
> On 12/6/18 8:35 AM, Matt Farmer wrote:
> > Ah, good point.
> >
> > Should we consider altering the serializer interface to permit not
> sending
> > the record?
> >
> > On Wed, Dec 5, 2018 at 9:23 PM Kamal Chandraprakash <
> > kamal.chandraprak...@gmail.com> wrote:
> >
> >> Matt,
> >>
> >> That's a good point. If these cases are handled in the serializer,
> then
> >> one cannot continue the stream processing by skipping the record.
> >> To continue, you may have to send a empty record serialized key/value
> (new
> >> byte[0]) to the downstream on hitting the error which may cause
> un-intended
> >> results.
> >>
> >>
> >>
> >>
> >>
> >> On Wed, Dec 5, 2018 at 8:41 PM Matt Farmer <m...@frmr.me> wrote:
> >>
> >>> Hi there,
> >>>
> >>> Thanks for this KIP.
> >>>
> >>> What’s the thinking behind doing this in ProductionExceptionHandler
> >> versus
> >>> handling these cases in your serializer implementation?
> >>>
> >>> On Mon, Dec 3, 2018 at 1:09 AM Kamal Chandraprakash <
> >>> kamal.chandraprak...@gmail.com> wrote:
> >>>
> >>>> Hello dev,
> >>>>
> >>>> I hope to initiate the discussion for KIP-399: Extend
> >>>> ProductionExceptionHandler to cover serialization exceptions.
> >>>>
> >>>> KIP: <
> >>>>
> >>>>
> >>>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-399%3A+Extend+ProductionExceptionHandler+to+cover+serialization+exceptions
> >>>>>
> >>>> JIRA: https://issues.apache.org/jira/browse/KAFKA-7499
> >>>>
> >>>> All feedbacks will be highly appreciated.
> >>>>
> >>>> Thanks,
> >>>> Kamal Chandraprakash
> >>>>
> >>>
> >>
> >
>
>
