Kafka
  1. Kafka
  2. KAFKA-251

The ConsumerStats MBean's PartOwnerStats attribute is a string

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • 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

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

          People

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

            Dates

            • Created:
              Updated:

              Development