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

2017-03-30 Thread Mayuresh Gharat
Hi Ismael,

I have updated the KIP. Let me know if everything looks fine then I will
begin voting.

Thanks,

Mayuresh

On Wed, Mar 29, 2017 at 9:06 AM, Mayuresh Gharat  wrote:

> Hi Ismael,
>
> I agree. I will change the compatibility para and start voting.
>
> Thanks,
>
> Mayuresh
>
> On Tue, Mar 28, 2017 at 6:40 PM, Ismael Juma  wrote:
>
>> Hi,
>>
>> I think error messages and error codes serve different purposes. Error
>> messages provide additional information about the error, but users should
>> never have to match on a message to handle an error/exception. For this
>> case, it seems like this is a fatal error so we could get away with just
>> using an error message. Having said that, InvalidKeyError is not too
>> specific and I'm OK with that too.
>>
>> As I said earlier, I do think that we need to change the following
>>
>> "It is recommended that we upgrade the clients before the broker is
>> upgraded, so that the clients would be able to understand the new
>> exception."
>>
>> This is problematic since we want older clients to work with newer
>> brokers.
>> That's why I recommended that we only throw this error if the
>> ProduceRequest is version 3 or higher.
>>
>> Ismael
>>
>> P.S. Note that we already send error messages back for the CreateTopics
>> protocol API (I added that in the previous release).
>>
>> On Tue, Mar 28, 2017 at 7:22 AM, Mayuresh Gharat <
>> gharatmayures...@gmail.com
>> > wrote:
>>
>> > I think, it's OK to do this right now.
>> > The other KIP will have a wider base to cover as it will include other
>> > exceptions as well and will take time.
>> >
>> > Thanks,
>> >
>> > Mayuresh
>> >
>> > On Mon, Mar 27, 2017 at 11:20 PM Dong Lin  wrote:
>> >
>> > > Sorry, I forget that you have mentioned this idea in your previous
>> > reply. I
>> > > guess the question is, do we still need this KIP if we can have custom
>> > > error message specified in the exception via the other KIP?
>> > >
>> > >
>> > > On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
>> > > gharatmayures...@gmail.com> wrote:
>> > >
>> > > > Hi Dong,
>> > > >
>> > > > I do agree with that as I said before the thought did cross my mind
>> > and I
>> > > > am working on getting another KIP ready to have error responses
>> > returned
>> > > > back to the client.
>> > > >
>> > > > In my opinion, it's OK to add a new error code if it justifies the
>> > need.
>> > > As
>> > > > Ismael, mentioned on the jira, we need a specific non retriable
>> error
>> > > code
>> > > > in this case, with specific message, at least until the other KIP is
>> > > ready.
>> > > >
>> > > > Thanks,
>> > > >
>> > > > Mayuresh
>> > > > On Mon, Mar 27, 2017 at 10:55 PM Dong Lin 
>> wrote:
>> > > >
>> > > > > Hey Mayuresh,
>> > > > >
>> > > > > I get that you want to provide a more specific error message to
>> user.
>> > > > Then
>> > > > > would it be more useful to have a KIP that allows custom error
>> > message
>> > > to
>> > > > > be returned to client together with the exception in the response?
>> > For
>> > > > > example, broker can include in the response
>> > > PolicyViolationException("key
>> > > > > can not be null for non-compact topic ${topic}") and client can
>> print
>> > > > this
>> > > > > error message in the log. My concern with current KIP that it is
>> not
>> > > very
>> > > > > scalable to always have a KIP and class for every new error we may
>> > see
>> > > in
>> > > > > the future. The list of error classes, and the errors that need
>> to be
>> > > > > caught and handled by the client code, will increase overtime and
>> > > become
>> > > > > harder to maintain.
>> > > > >
>> > > > > Thanks,
>> > > > > Dong
>> > > > >
>> > > > >
>> > > > > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
>> > > > > gharatmayures...@gmail.com
>> > > > > > wrote:
>> > > > >
>> > > > > > Hi Dong,
>> > > > > >
>> > > > > > I had thought about this before and wanted to do similar thing.
>> But
>> > > as
>> > > > > was
>> > > > > > pointed out in the jira ticket, we wanted something more
>> specific
>> > > than
>> > > > > > general.
>> > > > > > The main issue is that we do not propagate server side error
>> > messages
>> > > > to
>> > > > > > clients, right now. I am working on a KIP proposal to propose
>> it.
>> > > > > >
>> > > > > > Thanks,
>> > > > > >
>> > > > > > Mayuresh
>> > > > > >
>> > > > > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin 
>> > > wrote:
>> > > > > >
>> > > > > > > Hey Mayuresh,
>> > > > > > >
>> > > > > > > Thanks for the patch. I am wondering if it would be better to
>> > add a
>> > > > > more
>> > > > > > > general error, e.g. InvalidMessageException. The benefit is
>> that
>> > we
>> > > > can
>> > > > > > > reuse this for other message level error instead of adding one
>> > > > > exception
>> > > > > > > class for each possible exception in the future. This is
>> similar
>> > to
>> > > > the
>> > > > > > use
>> > > > > > > of InvalidRequestException. For example, ListOffsetResponse
>> may
>> > > > return
>> > > > > > 

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

2017-03-29 Thread Mayuresh Gharat
Hi Ismael,

I agree. I will change the compatibility para and start voting.

Thanks,

Mayuresh

On Tue, Mar 28, 2017 at 6:40 PM, Ismael Juma  wrote:

> Hi,
>
> I think error messages and error codes serve different purposes. Error
> messages provide additional information about the error, but users should
> never have to match on a message to handle an error/exception. For this
> case, it seems like this is a fatal error so we could get away with just
> using an error message. Having said that, InvalidKeyError is not too
> specific and I'm OK with that too.
>
> As I said earlier, I do think that we need to change the following
>
> "It is recommended that we upgrade the clients before the broker is
> upgraded, so that the clients would be able to understand the new
> exception."
>
> This is problematic since we want older clients to work with newer brokers.
> That's why I recommended that we only throw this error if the
> ProduceRequest is version 3 or higher.
>
> Ismael
>
> P.S. Note that we already send error messages back for the CreateTopics
> protocol API (I added that in the previous release).
>
> On Tue, Mar 28, 2017 at 7:22 AM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > I think, it's OK to do this right now.
> > The other KIP will have a wider base to cover as it will include other
> > exceptions as well and will take time.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Mon, Mar 27, 2017 at 11:20 PM Dong Lin  wrote:
> >
> > > Sorry, I forget that you have mentioned this idea in your previous
> > reply. I
> > > guess the question is, do we still need this KIP if we can have custom
> > > error message specified in the exception via the other KIP?
> > >
> > >
> > > On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
> > > gharatmayures...@gmail.com> wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > I do agree with that as I said before the thought did cross my mind
> > and I
> > > > am working on getting another KIP ready to have error responses
> > returned
> > > > back to the client.
> > > >
> > > > In my opinion, it's OK to add a new error code if it justifies the
> > need.
> > > As
> > > > Ismael, mentioned on the jira, we need a specific non retriable error
> > > code
> > > > in this case, with specific message, at least until the other KIP is
> > > ready.
> > > >
> > > > Thanks,
> > > >
> > > > Mayuresh
> > > > On Mon, Mar 27, 2017 at 10:55 PM Dong Lin 
> wrote:
> > > >
> > > > > Hey Mayuresh,
> > > > >
> > > > > I get that you want to provide a more specific error message to
> user.
> > > > Then
> > > > > would it be more useful to have a KIP that allows custom error
> > message
> > > to
> > > > > be returned to client together with the exception in the response?
> > For
> > > > > example, broker can include in the response
> > > PolicyViolationException("key
> > > > > can not be null for non-compact topic ${topic}") and client can
> print
> > > > this
> > > > > error message in the log. My concern with current KIP that it is
> not
> > > very
> > > > > scalable to always have a KIP and class for every new error we may
> > see
> > > in
> > > > > the future. The list of error classes, and the errors that need to
> be
> > > > > caught and handled by the client code, will increase overtime and
> > > become
> > > > > harder to maintain.
> > > > >
> > > > > Thanks,
> > > > > Dong
> > > > >
> > > > >
> > > > > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> > > > > gharatmayures...@gmail.com
> > > > > > wrote:
> > > > >
> > > > > > Hi Dong,
> > > > > >
> > > > > > I had thought about this before and wanted to do similar thing.
> But
> > > as
> > > > > was
> > > > > > pointed out in the jira ticket, we wanted something more specific
> > > than
> > > > > > general.
> > > > > > The main issue is that we do not propagate server side error
> > messages
> > > > to
> > > > > > clients, right now. I am working on a KIP proposal to propose it.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Mayuresh
> > > > > >
> > > > > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin 
> > > wrote:
> > > > > >
> > > > > > > Hey Mayuresh,
> > > > > > >
> > > > > > > Thanks for the patch. I am wondering if it would be better to
> > add a
> > > > > more
> > > > > > > general error, e.g. InvalidMessageException. The benefit is
> that
> > we
> > > > can
> > > > > > > reuse this for other message level error instead of adding one
> > > > > exception
> > > > > > > class for each possible exception in the future. This is
> similar
> > to
> > > > the
> > > > > > use
> > > > > > > of InvalidRequestException. For example, ListOffsetResponse may
> > > > return
> > > > > > > InvalidRequestException if duplicate partitions are found in
> the
> > > > > > > ListOffsetRequest. We don't return DuplicatedPartitionException
> > in
> > > > this
> > > > > > > case.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Dong
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> 

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

2017-03-28 Thread Ismael Juma
Hi,

I think error messages and error codes serve different purposes. Error
messages provide additional information about the error, but users should
never have to match on a message to handle an error/exception. For this
case, it seems like this is a fatal error so we could get away with just
using an error message. Having said that, InvalidKeyError is not too
specific and I'm OK with that too.

As I said earlier, I do think that we need to change the following

"It is recommended that we upgrade the clients before the broker is
upgraded, so that the clients would be able to understand the new
exception."

This is problematic since we want older clients to work with newer brokers.
That's why I recommended that we only throw this error if the
ProduceRequest is version 3 or higher.

Ismael

P.S. Note that we already send error messages back for the CreateTopics
protocol API (I added that in the previous release).

On Tue, Mar 28, 2017 at 7:22 AM, Mayuresh Gharat  wrote:

> I think, it's OK to do this right now.
> The other KIP will have a wider base to cover as it will include other
> exceptions as well and will take time.
>
> Thanks,
>
> Mayuresh
>
> On Mon, Mar 27, 2017 at 11:20 PM Dong Lin  wrote:
>
> > Sorry, I forget that you have mentioned this idea in your previous
> reply. I
> > guess the question is, do we still need this KIP if we can have custom
> > error message specified in the exception via the other KIP?
> >
> >
> > On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> > > Hi Dong,
> > >
> > > I do agree with that as I said before the thought did cross my mind
> and I
> > > am working on getting another KIP ready to have error responses
> returned
> > > back to the client.
> > >
> > > In my opinion, it's OK to add a new error code if it justifies the
> need.
> > As
> > > Ismael, mentioned on the jira, we need a specific non retriable error
> > code
> > > in this case, with specific message, at least until the other KIP is
> > ready.
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > > On Mon, Mar 27, 2017 at 10:55 PM Dong Lin  wrote:
> > >
> > > > Hey Mayuresh,
> > > >
> > > > I get that you want to provide a more specific error message to user.
> > > Then
> > > > would it be more useful to have a KIP that allows custom error
> message
> > to
> > > > be returned to client together with the exception in the response?
> For
> > > > example, broker can include in the response
> > PolicyViolationException("key
> > > > can not be null for non-compact topic ${topic}") and client can print
> > > this
> > > > error message in the log. My concern with current KIP that it is not
> > very
> > > > scalable to always have a KIP and class for every new error we may
> see
> > in
> > > > the future. The list of error classes, and the errors that need to be
> > > > caught and handled by the client code, will increase overtime and
> > become
> > > > harder to maintain.
> > > >
> > > > Thanks,
> > > > Dong
> > > >
> > > >
> > > > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> > > > gharatmayures...@gmail.com
> > > > > wrote:
> > > >
> > > > > Hi Dong,
> > > > >
> > > > > I had thought about this before and wanted to do similar thing. But
> > as
> > > > was
> > > > > pointed out in the jira ticket, we wanted something more specific
> > than
> > > > > general.
> > > > > The main issue is that we do not propagate server side error
> messages
> > > to
> > > > > clients, right now. I am working on a KIP proposal to propose it.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Mayuresh
> > > > >
> > > > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin 
> > wrote:
> > > > >
> > > > > > Hey Mayuresh,
> > > > > >
> > > > > > Thanks for the patch. I am wondering if it would be better to
> add a
> > > > more
> > > > > > general error, e.g. InvalidMessageException. The benefit is that
> we
> > > can
> > > > > > reuse this for other message level error instead of adding one
> > > > exception
> > > > > > class for each possible exception in the future. This is similar
> to
> > > the
> > > > > use
> > > > > > of InvalidRequestException. For example, ListOffsetResponse may
> > > return
> > > > > > InvalidRequestException if duplicate partitions are found in the
> > > > > > ListOffsetRequest. We don't return DuplicatedPartitionException
> in
> > > this
> > > > > > case.
> > > > > >
> > > > > > Thanks,
> > > > > > Dong
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > > > > > gharatmayures...@gmail.com
> > > > > > > wrote:
> > > > > >
> > > > > > > Hi All,
> > > > > > >
> > > > > > > We have created KIP-135 to propose that Kafka should return a
> > > > > > non-retriable
> > > > > > > error when the producer produces a message with null key to a
> log
> > > > > > compacted
> > > > > > > topic.
> > > > > > >
> > > > > > > Please find the KIP wiki in the link :
> > > > > > >
> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>

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

2017-03-27 Thread Mayuresh Gharat
I think, it's OK to do this right now.
The other KIP will have a wider base to cover as it will include other
exceptions as well and will take time.

Thanks,

Mayuresh

On Mon, Mar 27, 2017 at 11:20 PM Dong Lin  wrote:

> Sorry, I forget that you have mentioned this idea in your previous reply. I
> guess the question is, do we still need this KIP if we can have custom
> error message specified in the exception via the other KIP?
>
>
> On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com> wrote:
>
> > Hi Dong,
> >
> > I do agree with that as I said before the thought did cross my mind and I
> > am working on getting another KIP ready to have error responses returned
> > back to the client.
> >
> > In my opinion, it's OK to add a new error code if it justifies the need.
> As
> > Ismael, mentioned on the jira, we need a specific non retriable error
> code
> > in this case, with specific message, at least until the other KIP is
> ready.
> >
> > Thanks,
> >
> > Mayuresh
> > On Mon, Mar 27, 2017 at 10:55 PM Dong Lin  wrote:
> >
> > > Hey Mayuresh,
> > >
> > > I get that you want to provide a more specific error message to user.
> > Then
> > > would it be more useful to have a KIP that allows custom error message
> to
> > > be returned to client together with the exception in the response? For
> > > example, broker can include in the response
> PolicyViolationException("key
> > > can not be null for non-compact topic ${topic}") and client can print
> > this
> > > error message in the log. My concern with current KIP that it is not
> very
> > > scalable to always have a KIP and class for every new error we may see
> in
> > > the future. The list of error classes, and the errors that need to be
> > > caught and handled by the client code, will increase overtime and
> become
> > > harder to maintain.
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> > > gharatmayures...@gmail.com
> > > > wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > I had thought about this before and wanted to do similar thing. But
> as
> > > was
> > > > pointed out in the jira ticket, we wanted something more specific
> than
> > > > general.
> > > > The main issue is that we do not propagate server side error messages
> > to
> > > > clients, right now. I am working on a KIP proposal to propose it.
> > > >
> > > > Thanks,
> > > >
> > > > Mayuresh
> > > >
> > > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin 
> wrote:
> > > >
> > > > > Hey Mayuresh,
> > > > >
> > > > > Thanks for the patch. I am wondering if it would be better to add a
> > > more
> > > > > general error, e.g. InvalidMessageException. The benefit is that we
> > can
> > > > > reuse this for other message level error instead of adding one
> > > exception
> > > > > class for each possible exception in the future. This is similar to
> > the
> > > > use
> > > > > of InvalidRequestException. For example, ListOffsetResponse may
> > return
> > > > > InvalidRequestException if duplicate partitions are found in the
> > > > > ListOffsetRequest. We don't return DuplicatedPartitionException in
> > this
> > > > > case.
> > > > >
> > > > > Thanks,
> > > > > Dong
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > > > > gharatmayures...@gmail.com
> > > > > > wrote:
> > > > >
> > > > > > Hi All,
> > > > > >
> > > > > > We have created KIP-135 to propose that Kafka should return a
> > > > > non-retriable
> > > > > > error when the producer produces a message with null key to a log
> > > > > compacted
> > > > > > topic.
> > > > > >
> > > > > > Please find the KIP wiki in the link :
> > > > > >
> > > > > > 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.
> > > > > >
> > > > > >
> > > > > > We would love to hear your comments and suggestions.
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Mayuresh
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > -Regards,
> > > > Mayuresh R. Gharat
> > > > (862) 250-7125
> > > >
> > >
> >
>


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

2017-03-27 Thread Dong Lin
Sorry, I forget that you have mentioned this idea in your previous reply. I
guess the question is, do we still need this KIP if we can have custom
error message specified in the exception via the other KIP?


On Mon, Mar 27, 2017 at 11:00 PM, Mayuresh Gharat <
gharatmayures...@gmail.com> wrote:

> Hi Dong,
>
> I do agree with that as I said before the thought did cross my mind and I
> am working on getting another KIP ready to have error responses returned
> back to the client.
>
> In my opinion, it's OK to add a new error code if it justifies the need. As
> Ismael, mentioned on the jira, we need a specific non retriable error code
> in this case, with specific message, at least until the other KIP is ready.
>
> Thanks,
>
> Mayuresh
> On Mon, Mar 27, 2017 at 10:55 PM Dong Lin  wrote:
>
> > Hey Mayuresh,
> >
> > I get that you want to provide a more specific error message to user.
> Then
> > would it be more useful to have a KIP that allows custom error message to
> > be returned to client together with the exception in the response? For
> > example, broker can include in the response PolicyViolationException("key
> > can not be null for non-compact topic ${topic}") and client can print
> this
> > error message in the log. My concern with current KIP that it is not very
> > scalable to always have a KIP and class for every new error we may see in
> > the future. The list of error classes, and the errors that need to be
> > caught and handled by the client code, will increase overtime and become
> > harder to maintain.
> >
> > Thanks,
> > Dong
> >
> >
> > On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > Hi Dong,
> > >
> > > I had thought about this before and wanted to do similar thing. But as
> > was
> > > pointed out in the jira ticket, we wanted something more specific than
> > > general.
> > > The main issue is that we do not propagate server side error messages
> to
> > > clients, right now. I am working on a KIP proposal to propose it.
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> > > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin  wrote:
> > >
> > > > Hey Mayuresh,
> > > >
> > > > Thanks for the patch. I am wondering if it would be better to add a
> > more
> > > > general error, e.g. InvalidMessageException. The benefit is that we
> can
> > > > reuse this for other message level error instead of adding one
> > exception
> > > > class for each possible exception in the future. This is similar to
> the
> > > use
> > > > of InvalidRequestException. For example, ListOffsetResponse may
> return
> > > > InvalidRequestException if duplicate partitions are found in the
> > > > ListOffsetRequest. We don't return DuplicatedPartitionException in
> this
> > > > case.
> > > >
> > > > Thanks,
> > > > Dong
> > > >
> > > >
> > > >
> > > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > > > gharatmayures...@gmail.com
> > > > > wrote:
> > > >
> > > > > Hi All,
> > > > >
> > > > > We have created KIP-135 to propose that Kafka should return a
> > > > non-retriable
> > > > > error when the producer produces a message with null key to a log
> > > > compacted
> > > > > topic.
> > > > >
> > > > > Please find the KIP wiki in the link :
> > > > >
> > > > > 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.
> > > > >
> > > > >
> > > > > We would love to hear your comments and suggestions.
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Mayuresh
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > -Regards,
> > > Mayuresh R. Gharat
> > > (862) 250-7125
> > >
> >
>


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

2017-03-27 Thread Mayuresh Gharat
Hi Dong,

I do agree with that as I said before the thought did cross my mind and I
am working on getting another KIP ready to have error responses returned
back to the client.

In my opinion, it's OK to add a new error code if it justifies the need. As
Ismael, mentioned on the jira, we need a specific non retriable error code
in this case, with specific message, at least until the other KIP is ready.

Thanks,

Mayuresh
On Mon, Mar 27, 2017 at 10:55 PM Dong Lin  wrote:

> Hey Mayuresh,
>
> I get that you want to provide a more specific error message to user. Then
> would it be more useful to have a KIP that allows custom error message to
> be returned to client together with the exception in the response? For
> example, broker can include in the response PolicyViolationException("key
> can not be null for non-compact topic ${topic}") and client can print this
> error message in the log. My concern with current KIP that it is not very
> scalable to always have a KIP and class for every new error we may see in
> the future. The list of error classes, and the errors that need to be
> caught and handled by the client code, will increase overtime and become
> harder to maintain.
>
> Thanks,
> Dong
>
>
> On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi Dong,
> >
> > I had thought about this before and wanted to do similar thing. But as
> was
> > pointed out in the jira ticket, we wanted something more specific than
> > general.
> > The main issue is that we do not propagate server side error messages to
> > clients, right now. I am working on a KIP proposal to propose it.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin  wrote:
> >
> > > Hey Mayuresh,
> > >
> > > Thanks for the patch. I am wondering if it would be better to add a
> more
> > > general error, e.g. InvalidMessageException. The benefit is that we can
> > > reuse this for other message level error instead of adding one
> exception
> > > class for each possible exception in the future. This is similar to the
> > use
> > > of InvalidRequestException. For example, ListOffsetResponse may return
> > > InvalidRequestException if duplicate partitions are found in the
> > > ListOffsetRequest. We don't return DuplicatedPartitionException in this
> > > case.
> > >
> > > Thanks,
> > > Dong
> > >
> > >
> > >
> > > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > > gharatmayures...@gmail.com
> > > > wrote:
> > >
> > > > Hi All,
> > > >
> > > > We have created KIP-135 to propose that Kafka should return a
> > > non-retriable
> > > > error when the producer produces a message with null key to a log
> > > compacted
> > > > topic.
> > > >
> > > > Please find the KIP wiki in the link :
> > > >
> > > > 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.
> > > >
> > > >
> > > > We would love to hear your comments and suggestions.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Mayuresh
> > > >
> > >
> >
> >
> >
> > --
> > -Regards,
> > Mayuresh R. Gharat
> > (862) 250-7125
> >
>


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

2017-03-27 Thread Dong Lin
Hey Mayuresh,

I get that you want to provide a more specific error message to user. Then
would it be more useful to have a KIP that allows custom error message to
be returned to client together with the exception in the response? For
example, broker can include in the response PolicyViolationException("key
can not be null for non-compact topic ${topic}") and client can print this
error message in the log. My concern with current KIP that it is not very
scalable to always have a KIP and class for every new error we may see in
the future. The list of error classes, and the errors that need to be
caught and handled by the client code, will increase overtime and become
harder to maintain.

Thanks,
Dong


On Mon, Mar 27, 2017 at 7:20 PM, Mayuresh Gharat  wrote:

> Hi Dong,
>
> I had thought about this before and wanted to do similar thing. But as was
> pointed out in the jira ticket, we wanted something more specific than
> general.
> The main issue is that we do not propagate server side error messages to
> clients, right now. I am working on a KIP proposal to propose it.
>
> Thanks,
>
> Mayuresh
>
> On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin  wrote:
>
> > Hey Mayuresh,
> >
> > Thanks for the patch. I am wondering if it would be better to add a more
> > general error, e.g. InvalidMessageException. The benefit is that we can
> > reuse this for other message level error instead of adding one exception
> > class for each possible exception in the future. This is similar to the
> use
> > of InvalidRequestException. For example, ListOffsetResponse may return
> > InvalidRequestException if duplicate partitions are found in the
> > ListOffsetRequest. We don't return DuplicatedPartitionException in this
> > case.
> >
> > Thanks,
> > Dong
> >
> >
> >
> > On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > Hi All,
> > >
> > > We have created KIP-135 to propose that Kafka should return a
> > non-retriable
> > > error when the producer produces a message with null key to a log
> > compacted
> > > topic.
> > >
> > > Please find the KIP wiki in the link :
> > >
> > > 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.
> > >
> > >
> > > We would love to hear your comments and suggestions.
> > >
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>


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

2017-03-27 Thread Mayuresh Gharat
Hi Dong,

I had thought about this before and wanted to do similar thing. But as was
pointed out in the jira ticket, we wanted something more specific than
general.
The main issue is that we do not propagate server side error messages to
clients, right now. I am working on a KIP proposal to propose it.

Thanks,

Mayuresh

On Mon, Mar 27, 2017 at 5:55 PM, Dong Lin  wrote:

> Hey Mayuresh,
>
> Thanks for the patch. I am wondering if it would be better to add a more
> general error, e.g. InvalidMessageException. The benefit is that we can
> reuse this for other message level error instead of adding one exception
> class for each possible exception in the future. This is similar to the use
> of InvalidRequestException. For example, ListOffsetResponse may return
> InvalidRequestException if duplicate partitions are found in the
> ListOffsetRequest. We don't return DuplicatedPartitionException in this
> case.
>
> Thanks,
> Dong
>
>
>
> On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi All,
> >
> > We have created KIP-135 to propose that Kafka should return a
> non-retriable
> > error when the producer produces a message with null key to a log
> compacted
> > topic.
> >
> > Please find the KIP wiki in the link :
> >
> > 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.
> >
> >
> > We would love to hear your comments and suggestions.
> >
> >
> > Thanks,
> >
> > Mayuresh
> >
>



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


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

2017-03-27 Thread Dong Lin
Hey Mayuresh,

Thanks for the patch. I am wondering if it would be better to add a more
general error, e.g. InvalidMessageException. The benefit is that we can
reuse this for other message level error instead of adding one exception
class for each possible exception in the future. This is similar to the use
of InvalidRequestException. For example, ListOffsetResponse may return
InvalidRequestException if duplicate partitions are found in the
ListOffsetRequest. We don't return DuplicatedPartitionException in this
case.

Thanks,
Dong



On Wed, Mar 22, 2017 at 3:07 PM, Mayuresh Gharat  wrote:

> Hi All,
>
> We have created KIP-135 to propose that Kafka should return a non-retriable
> error when the producer produces a message with null key to a log compacted
> topic.
>
> Please find the KIP wiki in the link :
>
> 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.
>
>
> We would love to hear your comments and suggestions.
>
>
> Thanks,
>
> Mayuresh
>


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

2017-03-27 Thread Ismael Juma
Hi Mayuresh,

The relevant PR was merged last Friday. :) Starting the vote is fine by me.

Ismael

On Mon, Mar 27, 2017 at 5:49 PM, Mayuresh Gharat  wrote:

> Hi Ismael,
>
> Sure we can do that. Just wanted to check on the timeline on when this can
> go in.
> I can wait till the new ProduceRequest gets in to trunk.
> On the other hand we can also support it in the existing code.
> I am fine either ways.
>
> Should I start Vote on this, so that we can get this approved?
>
> Thanks,
>
> Mayuresh
>
> On Wed, Mar 22, 2017 at 11:49 PM, Ismael Juma  wrote:
>
> > Thanks for the KIP Mayuresh. I suggest we only throw this error for
> > ProduceRequest version 3, which is being introduced with KIP-98
> > (Exactly-once). That way, the compatibility story is clearer, in my
> > opinion.
> >
> > Ismael
> >
> > On Wed, Mar 22, 2017 at 10:07 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> > > Hi All,
> > >
> > > We have created KIP-135 to propose that Kafka should return a
> > non-retriable
> > > error when the producer produces a message with null key to a log
> > compacted
> > > topic.
> > >
> > > Please find the KIP wiki in the link :
> > >
> > > 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.
> > >
> > >
> > > We would love to hear your comments and suggestions.
> > >
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>


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

2017-03-27 Thread Mayuresh Gharat
Hi Ismael,

Sure we can do that. Just wanted to check on the timeline on when this can
go in.
I can wait till the new ProduceRequest gets in to trunk.
On the other hand we can also support it in the existing code.
I am fine either ways.

Should I start Vote on this, so that we can get this approved?

Thanks,

Mayuresh

On Wed, Mar 22, 2017 at 11:49 PM, Ismael Juma  wrote:

> Thanks for the KIP Mayuresh. I suggest we only throw this error for
> ProduceRequest version 3, which is being introduced with KIP-98
> (Exactly-once). That way, the compatibility story is clearer, in my
> opinion.
>
> Ismael
>
> On Wed, Mar 22, 2017 at 10:07 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com> wrote:
>
> > Hi All,
> >
> > We have created KIP-135 to propose that Kafka should return a
> non-retriable
> > error when the producer produces a message with null key to a log
> compacted
> > topic.
> >
> > Please find the KIP wiki in the link :
> >
> > 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.
> >
> >
> > We would love to hear your comments and suggestions.
> >
> >
> > Thanks,
> >
> > Mayuresh
> >
>



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


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

2017-03-22 Thread Ismael Juma
Thanks for the KIP Mayuresh. I suggest we only throw this error for
ProduceRequest version 3, which is being introduced with KIP-98
(Exactly-once). That way, the compatibility story is clearer, in my opinion.

Ismael

On Wed, Mar 22, 2017 at 10:07 PM, Mayuresh Gharat <
gharatmayures...@gmail.com> wrote:

> Hi All,
>
> We have created KIP-135 to propose that Kafka should return a non-retriable
> error when the producer produces a message with null key to a log compacted
> topic.
>
> Please find the KIP wiki in the link :
>
> 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.
>
>
> We would love to hear your comments and suggestions.
>
>
> Thanks,
>
> Mayuresh
>


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

2017-03-22 Thread Mayuresh Gharat
Hi James,

I meant that "it is recommended to upgrade clients before upgrading the
brokers".
Will update the KIP to reflect that.

Thanks,

Mayuresh

On Wed, Mar 22, 2017 at 4:42 PM, James Cheng  wrote:

> Mayuresh,
>
> The Compatibility/Migration section says to upgrade the clients first,
> before the brokers. Are you talking about implementation or deployment? Do
> you mean to implement the client changes before the broker changes? That
> would imply that it would take 2 Kafka releases to implement this KIP.
>
> Or, are you saying that when deploying this change, that you would
> recommend upgrading clients before upgrading brokers?
>
> Thanks,
> -James
>
>
> > On Mar 22, 2017, at 3:07 PM, Mayuresh Gharat 
> wrote:
> >
> > Hi All,
> >
> > We have created KIP-135 to propose that Kafka should return a
> non-retriable
> > error when the producer produces a message with null key to a log
> compacted
> > topic.
> >
> > Please find the KIP wiki in the link :
> >
> > 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.
> >
> >
> > We would love to hear your comments and suggestions.
> >
> >
> > Thanks,
> >
> > Mayuresh
>
>


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


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

2017-03-22 Thread James Cheng
Mayuresh,

The Compatibility/Migration section says to upgrade the clients first, before 
the brokers. Are you talking about implementation or deployment? Do you mean to 
implement the client changes before the broker changes? That would imply that 
it would take 2 Kafka releases to implement this KIP.

Or, are you saying that when deploying this change, that you would recommend 
upgrading clients before upgrading brokers?

Thanks,
-James


> On Mar 22, 2017, at 3:07 PM, Mayuresh Gharat  
> wrote:
> 
> Hi All,
> 
> We have created KIP-135 to propose that Kafka should return a non-retriable
> error when the producer produces a message with null key to a log compacted
> topic.
> 
> Please find the KIP wiki in the link :
> 
> 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.
> 
> 
> We would love to hear your comments and suggestions.
> 
> 
> Thanks,
> 
> Mayuresh