Ismael Juma created KAFKA-4635:
----------------------------------

             Summary: Client Compatibility follow-up
                 Key: KAFKA-4635
                 URL: https://issues.apache.org/jira/browse/KAFKA-4635
             Project: Kafka
          Issue Type: Sub-task
            Reporter: Ismael Juma


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.

cc [~hachikuji].




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to