[
https://issues.apache.org/jira/browse/KAFKA-4635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15827196#comment-15827196
]
Colin P. McCabe commented on KAFKA-4635:
----------------------------------------
bq. 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:
Agree
bq. 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.
How about {{OutdatedBrokerException}}?
bq. 3. Jun Rao 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.
It's a good idea. I would suggest logging at debug level, since otherwise it
will get extremely chatty whenever an older broker is around.
bq. 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.... We could infer
that the broker is too old if it happens when we send an ApiVersions request.
This should probably be a separate JIRA as it's not essential for the release.
Yeah, let's split that out into a separate JIRA. It's not a regression from
the current behavior, and it could be pretty difficult to implement. At best,
you are making a guess about what the disconnection means, when there could be
other causes. And some older versions had the bug where they don't disconnect
on the first request, but only on the second.
We discussed #5 and #6 already
bq. 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?
Sorry for the dumb question, but I don't completely understand the proposal.
The idea is to lower the priority of a log message that results from a failed
ApiVersionsRequest unless it is targetted at a bootstrap broker? Maybe it
would help to describe the specific configuration or situation where this would
be most useful.
bq. 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.
OK.
bq. 9. We should add a note to the upgrade notes specifying the change in
behaviour with regards to older broker versions.
Sure, will add to docs/upgrade.html
> 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)
