Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.2.0
    • Component/s: clients
    • Labels:
      None

      Description

      I collected a number of improvements that I think would be good to do before the release. Colin McCabe, 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. 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.

      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. Jason Gustafson 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 Jason Gustafson.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                cmccabe Colin McCabe
                Reporter:
                ijuma Ismael Juma
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: