[ 
https://issues.apache.org/jira/browse/KAFKA-4635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15826790#comment-15826790
 ] 

Jason Gustafson commented on KAFKA-4635:
----------------------------------------

[~ijuma] I think I'm convinced we should not do #6. The problem is that a 
broker on an older version could still support the newer message format, so 
returning null in that case would lead to inconsistent results between those 
brokers and newer brokers which support the time index and the new ListOffsets 
API. It seems preferable to raise an exception.

> 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)

Reply via email to