Hi Guozang, Thanks for the KIP, looks like a great improvement. +1 for me.
-Bill On Fri, Jul 26, 2019 at 8:18 PM Jun Rao <j...@confluent.io> wrote: > Hi, Guozhang, > > Thanks for the KIP. +1 > > Jun > > 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 > > > > > >
