Kafka
  1. Kafka
  2. KAFKA-78

add optional mx4j support to expose jmx over http

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7
    • Component/s: None
    • Labels:
      None

      Description

      In the fun world of operations and monitoring sometimes HTTP is a better choice than 'real' jmx for alerting. mx4j exposes jmx beans over HTTP (sadly with xml). Attached patch is a scala port of CASSANDRA-1068 that optionally loads mx4j if it is present on the classpath.

      1. k78-v1.txt
        4 kB
        Chris Burroughs
      2. k78-v2.txt
        4 kB
        Chris Burroughs
      3. k78-v3.txt
        5 kB
        Chris Burroughs
      4. k78-v4.txt
        4 kB
        Chris Burroughs

        Activity

        Hide
        Jay Kreps added a comment -

        This is nice. Couple of things:

        • Do we have anyone who would directly make use of this? Without that these things tend to rot...
        • Jun/Neha, we are already using mx4j what happens to someone already using mx4j if this code runs too?
        • We have tried to make the kafka broker easily embeddable. I wonder if we should not just let people add additional "container" functionality that way rather than us trying to add it all into the kafka itself. What I mean is you can easily make an XyzKafkaWrapper.java that does whatever custom logic you want in addition to starting the Kafka broker.
        Show
        Jay Kreps added a comment - This is nice. Couple of things: Do we have anyone who would directly make use of this? Without that these things tend to rot... Jun/Neha, we are already using mx4j what happens to someone already using mx4j if this code runs too? We have tried to make the kafka broker easily embeddable. I wonder if we should not just let people add additional "container" functionality that way rather than us trying to add it all into the kafka itself. What I mean is you can easily make an XyzKafkaWrapper.java that does whatever custom logic you want in addition to starting the Kafka broker.
        Hide
        Blake Matheny added a comment -

        I would use it. Right now all of our production JVM code exposes stats over HTTP, we mostly avoid JMX.

        On the embeddable broker, I'd sign up to help spec/dev it if needed. I asked if that was possible in the IRC channel last week and the short answer was, "not really".

        Show
        Blake Matheny added a comment - I would use it. Right now all of our production JVM code exposes stats over HTTP, we mostly avoid JMX. On the embeddable broker, I'd sign up to help spec/dev it if needed. I asked if that was possible in the IRC channel last week and the short answer was, "not really".
        Hide
        Jay Kreps added a comment -

        The kafka broker is just an object that takes a java.util.Properties in its constructor so it should be very easy to embed. Was there a reason it couldn't be embedded? We do this in tests and indeed this is how linkedin does it so i think it does work...

        I think this is an important use case, the question is do we want to try to build this in or let other people create their own wrappers. I could see going either way, but my experience is that a lot of this packaging stuff tends to be fairly environment specific so even if someone else is using mx4j they may have a different incompatible version so it is just better to let people embed and do their own thing. (Actually LinkedIn is using mx4j in this very way and I am a little concerned this would conflict with out usage...so it would be good if there was an option so it could be disabled at the least).

        So I think we should either:
        1. Document how to embed the broker to make it easy for people to do these kinds of things OR
        2. Make it so that this can be disabled

        Show
        Jay Kreps added a comment - The kafka broker is just an object that takes a java.util.Properties in its constructor so it should be very easy to embed. Was there a reason it couldn't be embedded? We do this in tests and indeed this is how linkedin does it so i think it does work... I think this is an important use case, the question is do we want to try to build this in or let other people create their own wrappers. I could see going either way, but my experience is that a lot of this packaging stuff tends to be fairly environment specific so even if someone else is using mx4j they may have a different incompatible version so it is just better to let people embed and do their own thing. (Actually LinkedIn is using mx4j in this very way and I am a little concerned this would conflict with out usage...so it would be good if there was an option so it could be disabled at the least). So I think we should either: 1. Document how to embed the broker to make it easy for people to do these kinds of things OR 2. Make it so that this can be disabled
        Hide
        Jun Rao added a comment -

        I think if this is disabled by default through a config, then it should be fine. Some minor comments on the patch:
        1. please remove unused import javax.management.MBeanServer
        2. fix typo scale -> scala

        Show
        Jun Rao added a comment - I think if this is disabled by default through a config, then it should be fine. Some minor comments on the patch: 1. please remove unused import javax.management.MBeanServer 2. fix typo scale -> scala
        Hide
        Chris Burroughs added a comment -

        I like having a clean separation between "this is the kafka class" and "this is the class for the kafka daemon (and will instantiate the kafka server class)".

        If you try to start mx4j twice you will get something like this in the logs, but I think everything will function correctly.

        [2011-08-08 15:34:43,470] WARN Could not start register mbean in JMX (kafka.utils.Mx4jLoader$)
        javax.management.InstanceAlreadyExistsException: system:name=http
        at com.sun.jmx.mbeanserver.Repository.addMBean(Repository.java:453)
        at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.internal_addObject(DefaultMBeanServerInterceptor.java:1484)
        at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerDynamicMBean(DefaultMBeanServerInterceptor.java:963)
        at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerObject(DefaultMBeanServerInterceptor.java:917)
        at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerMBean(DefaultMBeanServerInterceptor.java:312)
        at com.sun.jmx.mbeanserver.JmxMBeanServer.registerMBean(JmxMBeanServer.java:482)
        at kafka.utils.Mx4jLoader$.maybeLoad(Mx4jLoader.scala:47)
        at kafka.server.KafkaServer.startup(KafkaServer.scala:74)
        at kafka.server.KafkaServerStartable.startup(KafkaServerStartable.scala:40)
        at kafka.Kafka$.main(Kafka.scala:56)
        at kafka.Kafka.main(Kafka.scala)

        Show
        Chris Burroughs added a comment - I like having a clean separation between "this is the kafka class" and "this is the class for the kafka daemon (and will instantiate the kafka server class)". If you try to start mx4j twice you will get something like this in the logs, but I think everything will function correctly. [2011-08-08 15:34:43,470] WARN Could not start register mbean in JMX (kafka.utils.Mx4jLoader$) javax.management.InstanceAlreadyExistsException: system:name=http at com.sun.jmx.mbeanserver.Repository.addMBean(Repository.java:453) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.internal_addObject(DefaultMBeanServerInterceptor.java:1484) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerDynamicMBean(DefaultMBeanServerInterceptor.java:963) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerObject(DefaultMBeanServerInterceptor.java:917) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerMBean(DefaultMBeanServerInterceptor.java:312) at com.sun.jmx.mbeanserver.JmxMBeanServer.registerMBean(JmxMBeanServer.java:482) at kafka.utils.Mx4jLoader$.maybeLoad(Mx4jLoader.scala:47) at kafka.server.KafkaServer.startup(KafkaServer.scala:74) at kafka.server.KafkaServerStartable.startup(KafkaServerStartable.scala:40) at kafka.Kafka$.main(Kafka.scala:56) at kafka.Kafka.main(Kafka.scala)
        Hide
        Chris Burroughs added a comment -

        v2 fixes typos and imports

        Show
        Chris Burroughs added a comment - v2 fixes typos and imports
        Hide
        Chris Burroughs added a comment -

        Jay, are you satisfied with the multiple invitations of mx4j case?

        Show
        Chris Burroughs added a comment - Jay, are you satisfied with the multiple invitations of mx4j case?
        Hide
        Jay Kreps added a comment -

        Shouldn't the mx4j port come from kafka config, why make it a system property?

        What happens if two kafka servers are instantiated in the same jvm? We do this for testing quite a lot.

        We should also have a config to disable this feature.

        Show
        Jay Kreps added a comment - Shouldn't the mx4j port come from kafka config, why make it a system property? What happens if two kafka servers are instantiated in the same jvm? We do this for testing quite a lot. We should also have a config to disable this feature.
        Hide
        Jun Rao added a comment -

        It doesn't looks like that we have a patch that addresses all the concerns here. Perhaps we can defer this to 0.8?

        Show
        Jun Rao added a comment - It doesn't looks like that we have a patch that addresses all the concerns here. Perhaps we can defer this to 0.8?
        Hide
        Chris Burroughs added a comment -

        Sorry for the delay.

        • License header format updated.
        • Added -Dmx4jdisable to disable mx4j even if it's on the classpath.

        They are system properties because it only makes sense to have one mx4j port per JVM (like jmx ports), not one per embedded broker. If there is a case there you have a lot of embedded brokers, and mx4j (ie not a test), and the log messages are annoying I can add some more complicated "do only once" code.

        Unit tests should be unaffected since we don't have a hard dep on mx4j.

        Show
        Chris Burroughs added a comment - Sorry for the delay. License header format updated. Added -Dmx4jdisable to disable mx4j even if it's on the classpath. They are system properties because it only makes sense to have one mx4j port per JVM (like jmx ports), not one per embedded broker. If there is a case there you have a lot of embedded brokers, and mx4j (ie not a test), and the log messages are annoying I can add some more complicated "do only once" code. Unit tests should be unaffected since we don't have a hard dep on mx4j.
        Hide
        Jun Rao added a comment -

        Chris, thanks for the patch. Do you think that we can change mx4jdisable to mx4jenable so that if the user doesn't specify the property, the mx4j part is not touched? This way, users who already use mx4j but don't want mx4j to be loaded in the embedded Kafka broker are not affected by default.

        Show
        Jun Rao added a comment - Chris, thanks for the patch. Do you think that we can change mx4jdisable to mx4jenable so that if the user doesn't specify the property, the mx4j part is not touched? This way, users who already use mx4j but don't want mx4j to be loaded in the embedded Kafka broker are not affected by default.
        Hide
        Chris Burroughs added a comment -

        v4 defaults to off (even with mx4j is on the classpath).

        Show
        Chris Burroughs added a comment - v4 defaults to off (even with mx4j is on the classpath).
        Hide
        Jun Rao added a comment -

        +1. Thanks for the patch, Chris.

        Show
        Jun Rao added a comment - +1. Thanks for the patch, Chris.
        Hide
        Jun Rao added a comment - - edited

        Committed this with a minor change (rename property mx4jenable to kafka_mx4jenable). Thanks for the patch, Chris.

        Show
        Jun Rao added a comment - - edited Committed this with a minor change (rename property mx4jenable to kafka_mx4jenable). Thanks for the patch, Chris.

          People

          • Assignee:
            Chris Burroughs
            Reporter:
            Chris Burroughs
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development