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 >
