Kafka
  1. Kafka
  2. KAFKA-852

Remove clientId from OffsetFetchResponse and OffsetCommitResponse

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.8.1
    • Fix Version/s: 0.8.1
    • Component/s: core
    • Labels:
      None

      Description

      These are not needed and conflict with the API documentation. Should be removed to be consistent with other APIs

      1. KAFKA-852v2.diff
        6 kB
        David Arthur
      2. KAFKA-852.diff
        6 kB
        David Arthur
      3. 0001-KAFKA-852-remove-clientId-from-Offset-Fetch-Commit-R.patch
        6 kB
        David Arthur

        Activity

        Hide
        Jay Kreps added a comment -

        Sorry for lagging on this. Committed.

        Show
        Jay Kreps added a comment - Sorry for lagging on this. Committed.
        Hide
        David Arthur added a comment -

        Meant "diff" not "patch"

        Show
        David Arthur added a comment - Meant "diff" not "patch"
        Hide
        David Arthur added a comment -

        v2 of patch rebased against current trunk.

        Show
        David Arthur added a comment - v2 of patch rebased against current trunk.
        Hide
        Jun Rao added a comment -

        Sorry for the late review. The patch no longer applies to trunk. Could you rebase? Thanks,

        Show
        Jun Rao added a comment - Sorry for the late review. The patch no longer applies to trunk. Could you rebase? Thanks,
        Hide
        David Arthur added a comment -

        The responses should include correlation, but not client id. The client id is just used for logging purposes on the broker, but correlation is used for clients to do async request/response handling. From the protocol wiki page: "Response => CorrelationId ResponseMessage"

        I had incorrectly included the client id in these two new responses (I originally was including version id as well, which was also a mistake - KAFKA-759).

        https://cwiki.apache.org/confluence/display/KAFKA/A+Guide+To+The+Kafka+Protocol#AGuideToTheKafkaProtocol-Responses

        Show
        David Arthur added a comment - The responses should include correlation, but not client id. The client id is just used for logging purposes on the broker, but correlation is used for clients to do async request/response handling. From the protocol wiki page: "Response => CorrelationId ResponseMessage" I had incorrectly included the client id in these two new responses (I originally was including version id as well, which was also a mistake - KAFKA-759 ). https://cwiki.apache.org/confluence/display/KAFKA/A+Guide+To+The+Kafka+Protocol#AGuideToTheKafkaProtocol-Responses
        Hide
        Neha Narkhede added a comment -

        I think you meant to remove correlation id right ?

        Show
        Neha Narkhede added a comment - I think you meant to remove correlation id right ?
        Hide
        David Arthur added a comment -

        @Neha, I attached a diff created using git-diff. The previous patch was generated using git format-patch.

        Also, this issue is targeting 0.8.1 (which I'm assuming is trunk) not 0.8. The Offset fetch/commit APIs are not slated for the 0.8 release AFAIK

        Show
        David Arthur added a comment - @Neha, I attached a diff created using git-diff. The previous patch was generated using git format-patch. Also, this issue is targeting 0.8.1 (which I'm assuming is trunk) not 0.8. The Offset fetch/commit APIs are not slated for the 0.8 release AFAIK
        Hide
        Neha Narkhede added a comment - - edited

        Thanks for the patch, David. However, I'm not able to apply it using the following commands -

        patch -p1 kafka-852.patch
        patch -p0 kafka-852.patch

        Please create a patch using the following command -

        git diff <local-0.8-branch> > kafka-852.patch
        OR
        git remote update
        git diff origin/0.8 > kafka-852.patch

        Show
        Neha Narkhede added a comment - - edited Thanks for the patch, David. However, I'm not able to apply it using the following commands - patch -p1 kafka-852.patch patch -p0 kafka-852.patch Please create a patch using the following command - git diff <local-0.8-branch> > kafka-852.patch OR git remote update git diff origin/0.8 > kafka-852.patch
        Hide
        David Arthur added a comment -

        v1 of patch attached, simple change

        Show
        David Arthur added a comment - v1 of patch attached, simple change

          People

          • Assignee:
            David Arthur
            Reporter:
            David Arthur
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2h
              2h
              Remaining:
              Remaining Estimate - 2h
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development