Kafka
  1. Kafka
  2. KAFKA-251

The ConsumerStats MBean's PartOwnerStats attribute is a string

    Details

    • Type: Bug Bug
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      The fact that the PartOwnerStats is a string prevents monitoring systems from graphing consumer lag. There should be one mbean per [ topic, partition, groupid ] group.

        Activity

        Pierre-Yves Ritschard created issue -
        Pierre-Yves Ritschard made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Pierre-Yves Ritschard made changes -
        Hide
        Jun Rao added a comment -

        Thanks for the patch. Some comments:

        1. There is compilation error.
        2. Use space instead of TAB.
        3. The new jmx seems to be intended for replacing ZookeeperConsumerConnectorMBean. If so, we should remove ZookeeperConsumerConnectMBean and the implementation in ZookeeperConsumerConnector.
        4. Not every SyncRebalance triggers a successful rebalance (sometimes rebalance is not necessary). So, instead of unregistering/registering the MBean in SyncRebalance, we should do unregisterMBeans followed by registerMBeans at the end of updateFetcher, which is when a rebalance really happens.

        Show
        Jun Rao added a comment - Thanks for the patch. Some comments: 1. There is compilation error. 2. Use space instead of TAB. 3. The new jmx seems to be intended for replacing ZookeeperConsumerConnectorMBean. If so, we should remove ZookeeperConsumerConnectMBean and the implementation in ZookeeperConsumerConnector. 4. Not every SyncRebalance triggers a successful rebalance (sometimes rebalance is not necessary). So, instead of unregistering/registering the MBean in SyncRebalance, we should do unregisterMBeans followed by registerMBeans at the end of updateFetcher, which is when a rebalance really happens.
        Hide
        Pierre-Yves Ritschard added a comment -

        1 & 2: i must have sent a wrong patch, sorry about that, will reissue
        3: i thought some people may be relying on the actual output, but OK
        4: unregistermbeans needs to be called before topicRegistry is ever touched, or old beans will still exist, I will look at a better place to wrap this

        Thanks for the quick review, I'll update shortly

        Show
        Pierre-Yves Ritschard added a comment - 1 & 2: i must have sent a wrong patch, sorry about that, will reissue 3: i thought some people may be relying on the actual output, but OK 4: unregistermbeans needs to be called before topicRegistry is ever touched, or old beans will still exist, I will look at a better place to wrap this Thanks for the quick review, I'll update shortly
        Pierre-Yves Ritschard made changes -
        Hide
        Pierre-Yves Ritschard added a comment -

        I incorporated your comments safe for no 4 since i don't think it's too bad to sometimes unregister mbeans and reregister them, worse than can happen is a null value look up which graphing systems handle gracefully.

        Show
        Pierre-Yves Ritschard added a comment - I incorporated your comments safe for no 4 since i don't think it's too bad to sometimes unregister mbeans and reregister them, worse than can happen is a null value look up which graphing systems handle gracefully.
        Hide
        Jun Rao added a comment -

        Sorry, just get the chance to look at this. Couldn't apply the patch since trunk has moved. Could you rebase?

        Show
        Jun Rao added a comment - Sorry, just get the chance to look at this. Couldn't apply the patch since trunk has moved. Could you rebase?
        Hide
        Ismael Juma added a comment -

        Change to "In Progress" since the patch doesn't apply cleanly.

        Show
        Ismael Juma added a comment - Change to "In Progress" since the patch doesn't apply cleanly.
        Ismael Juma made changes -
        Status Patch Available [ 10002 ] In Progress [ 3 ]
        Hide
        Edward Ribeiro added a comment -

        Ismael Juma Hi, if it is still relevant and no one is working on it could you assign it to me, please?

        Show
        Edward Ribeiro added a comment - Ismael Juma Hi, if it is still relevant and no one is working on it could you assign it to me, please?
        Hide
        Ismael Juma added a comment -

        Edward Ribeiro, I don't know if it's still relevant as it's a pretty told ticket. Jun Rao should know. By the way, you should be able to assign tickets to yourself.

        Show
        Ismael Juma added a comment - Edward Ribeiro , I don't know if it's still relevant as it's a pretty told ticket. Jun Rao should know. By the way, you should be able to assign tickets to yourself.
        Hide
        Edward Ribeiro added a comment -

        I cannot. Afaik, it depends on the project, and some (many?) are restricting the assign operation to committers and core members, what I can understand perfectly and comply with.

        Show
        Edward Ribeiro added a comment - I cannot. Afaik, it depends on the project, and some (many?) are restricting the assign operation to committers and core members, what I can understand perfectly and comply with.
        Hide
        Ismael Juma added a comment -

        Edward Ribeiro, I am not a committer and I can. Ask in the mailing list to be added as a contributor for JIRA and Confluence (you can see many such messages in the archive).

        Show
        Ismael Juma added a comment - Edward Ribeiro , I am not a committer and I can. Ask in the mailing list to be added as a contributor for JIRA and Confluence (you can see many such messages in the archive).
        Hide
        Jun Rao added a comment -

        Since we are developing the new java consumer, not sure if it's worth patching the mbean in the old consumer.

        Show
        Jun Rao added a comment - Since we are developing the new java consumer, not sure if it's worth patching the mbean in the old consumer.
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        37s 1 Pierre-Yves Ritschard 24/Jan/12 17:09
        Patch Available Patch Available In Progress In Progress
        1273d 18h 28m 1 Ismael Juma 21/Jul/15 12:38

          People

          • Assignee:
            Unassigned
            Reporter:
            Pierre-Yves Ritschard
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development