Re: [DISCUSS] KIP-135 : Send of null key to a compacted topic should throw non-retriable error back to user
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
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
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
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
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
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
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
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
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
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
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
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
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
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