You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Mayuresh Gharat <gh...@gmail.com> on 2017/04/03 23:22:25 UTC

[VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Hi All,

It seems that there is no further concern with the KIP-135. At this point
we would like to start the voting process. The KIP can be found at
https://cwiki.apache.org/confluence/display/KAFKA/KIP-135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+non-retriable+error+back+to+user
<https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=67638388>

Thanks,

Mayuresh

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mathieu Fenniak <ma...@replicon.com>.
+1 (non-binding)


On Wed, Apr 5, 2017 at 10:45 AM, Mayuresh Gharat <gharatmayuresh15@gmail.com
> wrote:

> Bumping up this thread.
>
> On Mon, Apr 3, 2017 at 4:22 PM, Mayuresh Gharat <
> gharatmayuresh15@gmail.com>
> wrote:
>
> > Hi All,
> >
> > It seems that there is no further concern with the KIP-135. At this point
> > we would like to start the voting process. The KIP can be found at
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> > non-retriable+error+back+to+user
> > <https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=67638388>
> >
> > Thanks,
> >
> > Mayuresh
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mayuresh Gharat <gh...@gmail.com>.
Bumping up this thread.

On Mon, Apr 3, 2017 at 4:22 PM, Mayuresh Gharat <gh...@gmail.com>
wrote:

> Hi All,
>
> It seems that there is no further concern with the KIP-135. At this point
> we would like to start the voting process. The KIP can be found at
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> non-retriable+error+back+to+user
> <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=67638388>
>
> Thanks,
>
> Mayuresh
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mayuresh Gharat <gh...@gmail.com>.
Hi Ismael,

I went ahead and created a patch so that we get on same page regarding what
we want to do. We can make changes if you have  any comments.

Thanks,

Mayuresh

On Mon, Apr 10, 2017 at 11:32 AM, Mayuresh Gharat <
gharatmayuresh15@gmail.com> wrote:

> Got it.
> We can probably extend the InvalidRecordException with a more specific
> exception this use case and make it first class for produce side OR we can
> add an error code for InvalidRecordException in the Errors class and make
> it first class. I am fine either ways.
> What do you prefer?
>
> Thanks,
>
> Mayuresh
>
> On Mon, Apr 10, 2017 at 10:16 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
>> Hi Mayuresh,
>>
>> I was suggesting that we introduce a new error code for non retriable
>> invalid record exceptions (not sure what's a good name). We would then
>> change LogValidator and Log to use this new exception wherever it makes
>> sense (errors that are not retriable). One of many such cases is
>> https://github.com/apache/kafka/blob/5cf64f06a877a181d12a2ae2390516
>> ba1a572135/core/src/main/scala/kafka/log/LogValidator.scala#L78
>> <https://github.com/apache/kafka/blob/5cf64f06a877a181d12a2ae2390516ba1a572135/core/src/main/scala/kafka/log/LogValidator.scala#L78>
>>
>> Does that make sense?
>>
>> Ismael
>>
>> On Thu, Apr 6, 2017 at 5:50 PM, Mayuresh Gharat <
>> gharatmayuresh15@gmail.com>
>> wrote:
>>
>> > Hi Ismael,
>> >
>> > Are you suggesting to use the InvalidRecordException when the key is
>> null?
>> >
>> > Thanks,
>> >
>> > Mayuresh
>> >
>> > On Thu, Apr 6, 2017 at 8:49 AM, Ismael Juma <is...@juma.me.uk> wrote:
>> >
>> > > Hi Mayuresh,
>> > >
>> > > I took a closer look at the code and we seem to throw
>> > > `InvalidRecordException` in a number of cases where retrying doesn't
>> seem
>> > > to make sense. For example:
>> > >
>> > > throw new InvalidRecordException(s"Log record magic does not match
>> outer
>> > > magic ${batch.magic}")
>> > > throw new InvalidRecordException("Found invalid number of record
>> headers
>> > "
>> > > + numHeaders);
>> > > throw new InvalidRecordException("Found invalid record count " +
>> > numRecords
>> > > + " in magic v" + magic() + " batch");
>> > >
>> > > It seems like most of the usage of InvalidRecordException is for non
>> > > retriable errors. Maybe we need to introduce a non retriable version
>> of
>> > > this exception and use it in the various places where it makes sense.
>> > >
>> > > Ismael
>> > >
>> > > On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <
>> > > gharatmayuresh15@gmail.com
>> > > > wrote:
>> > >
>> > > > Hi All,
>> > > >
>> > > > It seems that there is no further concern with the KIP-135. At this
>> > point
>> > > > we would like to start the voting process. The KIP can be found at
>> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
>> > > > non-retriable+error+back+to+user
>> > > > <https://cwiki.apache.org/confluence/pages/viewpage.
>> > > action?pageId=67638388
>> > > > >
>> > > >
>> > > > Thanks,
>> > > >
>> > > > Mayuresh
>> > > >
>> > >
>> >
>> >
>> >
>> > --
>> > -Regards,
>> > Mayuresh R. Gharat
>> > (862) 250-7125
>> >
>>
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mayuresh Gharat <gh...@gmail.com>.
Got it.
We can probably extend the InvalidRecordException with a more specific
exception this use case and make it first class for produce side OR we can
add an error code for InvalidRecordException in the Errors class and make
it first class. I am fine either ways.
What do you prefer?

Thanks,

Mayuresh

On Mon, Apr 10, 2017 at 10:16 AM, Ismael Juma <is...@juma.me.uk> wrote:

> Hi Mayuresh,
>
> I was suggesting that we introduce a new error code for non retriable
> invalid record exceptions (not sure what's a good name). We would then
> change LogValidator and Log to use this new exception wherever it makes
> sense (errors that are not retriable). One of many such cases is
> https://github.com/apache/kafka/blob/5cf64f06a877a181d12a2ae2390516
> ba1a572135/core/src/main/scala/kafka/log/LogValidator.scala#L78
>
> Does that make sense?
>
> Ismael
>
> On Thu, Apr 6, 2017 at 5:50 PM, Mayuresh Gharat <
> gharatmayuresh15@gmail.com>
> wrote:
>
> > Hi Ismael,
> >
> > Are you suggesting to use the InvalidRecordException when the key is
> null?
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Thu, Apr 6, 2017 at 8:49 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Mayuresh,
> > >
> > > I took a closer look at the code and we seem to throw
> > > `InvalidRecordException` in a number of cases where retrying doesn't
> seem
> > > to make sense. For example:
> > >
> > > throw new InvalidRecordException(s"Log record magic does not match
> outer
> > > magic ${batch.magic}")
> > > throw new InvalidRecordException("Found invalid number of record
> headers
> > "
> > > + numHeaders);
> > > throw new InvalidRecordException("Found invalid record count " +
> > numRecords
> > > + " in magic v" + magic() + " batch");
> > >
> > > It seems like most of the usage of InvalidRecordException is for non
> > > retriable errors. Maybe we need to introduce a non retriable version of
> > > this exception and use it in the various places where it makes sense.
> > >
> > > Ismael
> > >
> > > On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <
> > > gharatmayuresh15@gmail.com
> > > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > It seems that there is no further concern with the KIP-135. At this
> > point
> > > > we would like to start the voting process. The KIP can be found at
> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> > > > non-retriable+error+back+to+user
> > > > <https://cwiki.apache.org/confluence/pages/viewpage.
> > > action?pageId=67638388
> > > > >
> > > >
> > > > Thanks,
> > > >
> > > > Mayuresh
> > > >
> > >
> >
> >
> >
> > --
> > -Regards,
> > Mayuresh R. Gharat
> > (862) 250-7125
> >
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Ismael Juma <is...@juma.me.uk>.
Hi Mayuresh,

I was suggesting that we introduce a new error code for non retriable
invalid record exceptions (not sure what's a good name). We would then
change LogValidator and Log to use this new exception wherever it makes
sense (errors that are not retriable). One of many such cases is
https://github.com/apache/kafka/blob/5cf64f06a877a181d12a2ae2390516
ba1a572135/core/src/main/scala/kafka/log/LogValidator.scala#L78

Does that make sense?

Ismael

On Thu, Apr 6, 2017 at 5:50 PM, Mayuresh Gharat <gh...@gmail.com>
wrote:

> Hi Ismael,
>
> Are you suggesting to use the InvalidRecordException when the key is null?
>
> Thanks,
>
> Mayuresh
>
> On Thu, Apr 6, 2017 at 8:49 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Mayuresh,
> >
> > I took a closer look at the code and we seem to throw
> > `InvalidRecordException` in a number of cases where retrying doesn't seem
> > to make sense. For example:
> >
> > throw new InvalidRecordException(s"Log record magic does not match outer
> > magic ${batch.magic}")
> > throw new InvalidRecordException("Found invalid number of record headers
> "
> > + numHeaders);
> > throw new InvalidRecordException("Found invalid record count " +
> numRecords
> > + " in magic v" + magic() + " batch");
> >
> > It seems like most of the usage of InvalidRecordException is for non
> > retriable errors. Maybe we need to introduce a non retriable version of
> > this exception and use it in the various places where it makes sense.
> >
> > Ismael
> >
> > On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <
> > gharatmayuresh15@gmail.com
> > > wrote:
> >
> > > Hi All,
> > >
> > > It seems that there is no further concern with the KIP-135. At this
> point
> > > we would like to start the voting process. The KIP can be found at
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> > > non-retriable+error+back+to+user
> > > <https://cwiki.apache.org/confluence/pages/viewpage.
> > action?pageId=67638388
> > > >
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mayuresh Gharat <gh...@gmail.com>.
Ping Ismael.

Thanks,

Mayuresh

On Thu, Apr 6, 2017 at 9:50 AM, Mayuresh Gharat <gh...@gmail.com>
wrote:

> Hi Ismael,
>
> Are you suggesting to use the InvalidRecordException when the key is null?
>
> Thanks,
>
> Mayuresh
>
> On Thu, Apr 6, 2017 at 8:49 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
>> Hi Mayuresh,
>>
>> I took a closer look at the code and we seem to throw
>> `InvalidRecordException` in a number of cases where retrying doesn't seem
>> to make sense. For example:
>>
>> throw new InvalidRecordException(s"Log record magic does not match outer
>> magic ${batch.magic}")
>> throw new InvalidRecordException("Found invalid number of record headers "
>> + numHeaders);
>> throw new InvalidRecordException("Found invalid record count " +
>> numRecords
>> + " in magic v" + magic() + " batch");
>>
>> It seems like most of the usage of InvalidRecordException is for non
>> retriable errors. Maybe we need to introduce a non retriable version of
>> this exception and use it in the various places where it makes sense.
>>
>> Ismael
>>
>> On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <
>> gharatmayuresh15@gmail.com
>> > wrote:
>>
>> > Hi All,
>> >
>> > It seems that there is no further concern with the KIP-135. At this
>> point
>> > we would like to start the voting process. The KIP can be found at
>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
>> > non-retriable+error+back+to+user
>> > <https://cwiki.apache.org/confluence/pages/viewpage.action?
>> pageId=67638388
>> > >
>> >
>> > Thanks,
>> >
>> > Mayuresh
>> >
>>
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Mayuresh Gharat <gh...@gmail.com>.
Hi Ismael,

Are you suggesting to use the InvalidRecordException when the key is null?

Thanks,

Mayuresh

On Thu, Apr 6, 2017 at 8:49 AM, Ismael Juma <is...@juma.me.uk> wrote:

> Hi Mayuresh,
>
> I took a closer look at the code and we seem to throw
> `InvalidRecordException` in a number of cases where retrying doesn't seem
> to make sense. For example:
>
> throw new InvalidRecordException(s"Log record magic does not match outer
> magic ${batch.magic}")
> throw new InvalidRecordException("Found invalid number of record headers "
> + numHeaders);
> throw new InvalidRecordException("Found invalid record count " + numRecords
> + " in magic v" + magic() + " batch");
>
> It seems like most of the usage of InvalidRecordException is for non
> retriable errors. Maybe we need to introduce a non retriable version of
> this exception and use it in the various places where it makes sense.
>
> Ismael
>
> On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <
> gharatmayuresh15@gmail.com
> > wrote:
>
> > Hi All,
> >
> > It seems that there is no further concern with the KIP-135. At this point
> > we would like to start the voting process. The KIP can be found at
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> > non-retriable+error+back+to+user
> > <https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=67638388
> > >
> >
> > Thanks,
> >
> > Mayuresh
> >
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [VOTE] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user

Posted by Ismael Juma <is...@juma.me.uk>.
Hi Mayuresh,

I took a closer look at the code and we seem to throw
`InvalidRecordException` in a number of cases where retrying doesn't seem
to make sense. For example:

throw new InvalidRecordException(s"Log record magic does not match outer
magic ${batch.magic}")
throw new InvalidRecordException("Found invalid number of record headers "
+ numHeaders);
throw new InvalidRecordException("Found invalid record count " + numRecords
+ " in magic v" + magic() + " batch");

It seems like most of the usage of InvalidRecordException is for non
retriable errors. Maybe we need to introduce a non retriable version of
this exception and use it in the various places where it makes sense.

Ismael

On Tue, Apr 4, 2017 at 12:22 AM, Mayuresh Gharat <gharatmayuresh15@gmail.com
> wrote:

> Hi All,
>
> It seems that there is no further concern with the KIP-135. At this point
> we would like to start the voting process. The KIP can be found at
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 135+%3A+Send+of+null+key+to+a+compacted+topic+should+throw+
> non-retriable+error+back+to+user
> <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=67638388
> >
>
> Thanks,
>
> Mayuresh
>