[ https://issues.apache.org/jira/browse/KAFKA-4635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15827057#comment-15827057 ]
Colin P. McCabe commented on KAFKA-4635: ---------------------------------------- Thanks for filling in the notes. #5 is implemented by the patch up at KAFKA-4630 For #4, I'm not sure if we can give a reasonable error message for pre-0.10.0 brokers, since the broker will simply disconnect us as soon as we attempt to invoke ApiVersionsRequest. We could add a test that the disconnection happens as expected. With regard to #9, what kind of note would be appropriate? > Client Compatibility follow-up > ------------------------------ > > Key: KAFKA-4635 > URL: https://issues.apache.org/jira/browse/KAFKA-4635 > Project: Kafka > Issue Type: Sub-task > Components: clients > Reporter: Ismael Juma > Assignee: Colin P. McCabe > Fix For: 0.10.2.0 > > > I collected a number of improvements that I think would be good to do before > the release. [~cmccabe], please correct if I got anything wrong and feel free > to move some items to separate JIRAs. > 1. OffsetAndTimestamp is a public class and the javadoc should only include > the behaviour that users will see. The following (or part of it) should > probably be a non-javadoc comment as it only happens internally: > "* The timestamp should never be negative, unless it is invalid. This could > happen when handling a response from a broker that doesn't support KIP-79." > 2. There was a bit of a discussion with regards to the name of the exception > that is thrown when a broker is too old. The current name is > ObsoleteBrokerException. We should decide on the name and then we should > update the relevant producer/consumer methods to mention it. > 3. [~junrao] suggested that it would be a good idea log when downgrading > requests as the behaviour can be a little different. We should decide the > right logging level and add this. > 4. We should have a system test against 0.9.0.1 brokers. We don't support it, > but we should ideally give a reasonable error message. > 5. It seems like `Fetcher.listOffset` could use `retrieveOffsetsByTimes` > instead of calling `sendListOffsetRequests` directly. I think that would be a > little better, but not sure if others disagree. > 6. [~hachikuji] suggested that a version mismatch in the `offsetsForTimes` > call should result in null entry in map instead of exception for consistency > with how we handle the unsupported message format case. I am adding this to > make sure we discuss it, but I am not actually sure that is what we should > do. Under normal circumstances, the brokers are either too old or not whereas > the message format is a topic level configuration and, strictly speaking, > independent of the broker version (there is a correlation in practice). > 7. We log a warning in case of an error while doing an ApiVersions request. > Because it is the first request and we retry, the warning in the log is > useful. We have a similar warning for Metadata requests, but we only did it > for bootstrap brokers. Would it make sense to do the same for ApiVersions? > 8. It would be good to add a few more tests for the usable versions > computation. We have a single simple one at the moment. > 9. We should add a note to the upgrade notes specifying the change in > behaviour with regards to older broker versions. > cc [~hachikuji]. -- This message was sent by Atlassian JIRA (v6.3.4#6332)