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

        David Arthur created issue -
        David Arthur made changes -
        Field Original Value New Value
        Attachment 0001-KAFKA-852-remove-clientId-from-Offset-Fetch-Commit-R.patch [ 12577257 ]
        Hide
        David Arthur added a comment -

        v1 of patch attached, simple change

        Show
        David Arthur added a comment - v1 of patch attached, simple change
        David Arthur made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        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
        David Arthur made changes -
        Attachment KAFKA-852.diff [ 12577573 ]
        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 -

        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 -

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

        v2 of patch rebased against current trunk.

        Show
        David Arthur added a comment - v2 of patch rebased against current trunk.
        David Arthur made changes -
        Attachment KAFKA-852v2.patch [ 12587817 ]
        Hide
        David Arthur added a comment -

        Meant "diff" not "patch"

        Show
        David Arthur added a comment - Meant "diff" not "patch"
        David Arthur made changes -
        Attachment KAFKA-852v2.diff [ 12587818 ]
        David Arthur made changes -
        Attachment KAFKA-852v2.patch [ 12587817 ]
        Hide
        Jay Kreps added a comment -

        Sorry for lagging on this. Committed.

        Show
        Jay Kreps added a comment - Sorry for lagging on this. Committed.
        Jay Kreps made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        4h 15m 1 David Arthur 05/Apr/13 19:41
        Patch Available Patch Available Resolved Resolved
        97d 3h 56m 1 Jay Kreps 11/Jul/13 23:37

          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