Yeah my thinking is that changing the return error code away from CORRUPTED_RECORD is really a bug fix, so we should just do it anyways without considering compatibility. I like returning INVALID_REQUEST too, would change it in the wiki.
Guozhang On Fri, Jul 26, 2019 at 4:40 PM Jason Gustafson <ja...@confluent.io> wrote: > Hi Guozhang, > > I agree it is misleading to suggest corruption in these cases. Have you > considered alternative error codes? I think INVALID_REQUEST may be more > suggestive that the server has rejected the request for some reason. > > In any case, it's a small point that need not block the KIP. I'm +1 > overall. > > Thanks, > Jason > > On Fri, Jul 26, 2019 at 4:24 PM Guozhang Wang <wangg...@gmail.com> wrote: > > > Hi Jason, thanks for the comments. > > > > 1. Yes that's a good point. Will move it to `errors`. > > > > 2. The point is that when broker returning the new error code > > INVALID_RECORD to the old versioned clients who do not recognize the > code, > > it would be translated to a UnknownServerException, whereas today > (without > > this KIP) the client would see CorruptRecordException that covers a bunch > > of scenarios that actually are not related to corrupted records at all. > > > > I feel that the new behavior is actually better, i.e. let clients report > an > > UnknownServerException rather than a more concrete, but incorrect > > CorruptRecordException. If we want to maintain compatibility we can let > > brokers to return the same error code to old versioned clients, but I'm > not > > sure if it is actually better. > > > > > > Guozhang > > > > On Thu, Jul 25, 2019 at 5:08 PM Jason Gustafson <ja...@confluent.io> > > wrote: > > > > > Hi Guozhang, > > > > > > The proposal looks good. A couple minor questions. > > > > > > 1. InvalidRecordException is currently located in > > > `org.apache.kafka.common.record`, which is not a public package. Shall > we > > > move it to `org.apache.kafka.common.errors`? > > > 2. I'm not sure I understand the point about UnknownServerException in > > the > > > compatibility section. Are you suggesting that we would use the new > error > > > code even for old versions of the produce request? > > > > > > Thanks, > > > Jason > > > > > > > > > > > > > > > On Tue, Jul 16, 2019 at 3:46 PM Guozhang Wang <wangg...@gmail.com> > > wrote: > > > > > > > Hello folks, > > > > > > > > I'd like to start a voting thread on KIP-467 to improve error > > > communication > > > > and handling for producer response: > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-467 > > > > > > %3A+Augment+ProduceResponse+error+messaging+for+specific+culprit+records > > > > > > > > > > > > Thanks, > > > > > > > > -- Guozhang > > > > > > > > > > > > > -- > > -- Guozhang > > > -- -- Guozhang
