ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-81

JMX module is using 1 java 6 method that has a java 5 equivalent

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0
    • Fix Version/s: 3.0.0
    • Component/s: server
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      It would be nice if the jmx module compiled and ran on java 5 too.

      1. ZOOKEEPER-81.patch
        0.6 kB
        Hiram Chirino

        Activity

        Hide
        Hiram Chirino added a comment -
        Index: src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java
        ===================================================================
        --- src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java	(revision 677957)
        +++ src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java	(working copy)
        @@ -143,7 +143,7 @@
             private int tokenize(StringBuilder sb, String path, int index){
                 String[] tokens = path.split("/");
                 for (String s: tokens) {
        -            if (s.isEmpty())
        +            if (s.length()==0)
                         continue;
                     sb.append("name").append(index++)
                             .append("=").append(s).append(",");
        
        Show
        Hiram Chirino added a comment - Index: src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java =================================================================== --- src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java (revision 677957) +++ src/java/jmx/org/apache/zookeeper/jmx/MBeanRegistry.java (working copy) @@ -143,7 +143,7 @@ private int tokenize(StringBuilder sb, String path, int index){ String [] tokens = path.split( "/" ); for ( String s: tokens) { - if (s.isEmpty()) + if (s.length()==0) continue ; sb.append( "name" ).append(index++) .append( "=" ).append(s).append( "," );
        Hide
        Hiram Chirino added a comment -

        attaching patch for the fix.

        Show
        Hiram Chirino added a comment - attaching patch for the fix.
        Hide
        Mahadev konar added a comment -

        i think the build.xml only allows building of jmx with 1.6? doesnt it?

        Show
        Mahadev konar added a comment - i think the build.xml only allows building of jmx with 1.6? doesnt it?
        Hide
        Andrew Kornev added a comment -

        The JMX instrumentation code relies on the MXBean feature that is available only since Java 6. The build.xml conditionally includes the JMX code only when compiled under Java 6.

        Show
        Andrew Kornev added a comment - The JMX instrumentation code relies on the MXBean feature that is available only since Java 6. The build.xml conditionally includes the JMX code only when compiled under Java 6.
        Hide
        Hiram Chirino added a comment -

        The build lets me build under 1.5 and if I apply that patch it compiles and tests fine. Perhaps that mxbean feature is only needed at runtime?

        Show
        Hiram Chirino added a comment - The build lets me build under 1.5 and if I apply that patch it compiles and tests fine. Perhaps that mxbean feature is only needed at runtime?
        Hide
        Andrew Kornev added a comment -

        I don't think you actually build the jmx code under 1.5 – build.xml silently excludes the java\jmx directory from the build.

        Show
        Andrew Kornev added a comment - I don't think you actually build the jmx code under 1.5 – build.xml silently excludes the java\jmx directory from the build.
        Hide
        Patrick Hunt added a comment -

        Assigning to Andrew for followup/resolution.

        Show
        Patrick Hunt added a comment - Assigning to Andrew for followup/resolution.
        Hide
        Hiram Chirino added a comment -

        Java 1.5 can compile this for sure with the given patch.

        Show
        Hiram Chirino added a comment - Java 1.5 can compile this for sure with the given patch.
        Hide
        Andrew Kornev added a comment -

        Long story short – MXBean is a runtime dependency. Some internal machinery for handling user defined MXBeans is missing in Java 5. For details please refer to http://java.sun.com/javase/6/docs/technotes/guides/jmx/enhancements.html. The second bullet on the top says:

        MXBeans have been added. MXBeans are MBeans that provide a convenient way to bundle related values together without requiring clients to be specially configured to handle the bundles. A defined set of MXBeans already existed in the J2SE 5.0 platform, but Java SE 6 introduces an API to allow you to program your own custom MXBeans.

        Although there is absolutely nothing wrong with your patch, would you be terribly disappointed if I leave the things the way they are and simply close this jira as "won't fix"?

        Show
        Andrew Kornev added a comment - Long story short – MXBean is a runtime dependency. Some internal machinery for handling user defined MXBeans is missing in Java 5. For details please refer to http://java.sun.com/javase/6/docs/technotes/guides/jmx/enhancements.html . The second bullet on the top says: MXBeans have been added. MXBeans are MBeans that provide a convenient way to bundle related values together without requiring clients to be specially configured to handle the bundles. A defined set of MXBeans already existed in the J2SE 5.0 platform, but Java SE 6 introduces an API to allow you to program your own custom MXBeans. Although there is absolutely nothing wrong with your patch, would you be terribly disappointed if I leave the things the way they are and simply close this jira as "won't fix"?
        Hide
        Hiram Chirino added a comment -

        lol.. I don't understand the aversion to the patch. it changes NOTHING significant and lets the module compile under java 1.5

        have you seen that it just changes ONE line? it changes "s.isEmpty()" to "s.length()==0"

        please commit.

        Show
        Hiram Chirino added a comment - lol.. I don't understand the aversion to the patch. it changes NOTHING significant and lets the module compile under java 1.5 have you seen that it just changes ONE line? it changes "s.isEmpty()" to "s.length()==0" please commit.
        Hide
        Hiram Chirino added a comment -

        Furthermore, I've been looking at your the MXBean interfaces and I don't see them being actual MXBeans.. they are just standard MBeans. They would only be MXBeans if the methods they defined used complex object types as parameters or return values.

        Are you sure that you even have a Java 6 runtime dependency?

        Show
        Hiram Chirino added a comment - Furthermore, I've been looking at your the MXBean interfaces and I don't see them being actual MXBeans.. they are just standard MBeans. They would only be MXBeans if the methods they defined used complex object types as parameters or return values. Are you sure that you even have a Java 6 runtime dependency?
        Hide
        Andrew Kornev added a comment -

        Would you like to verify whether or not there is a Java 6 runtime dependency? Would you be willing to contribute a patch (including this change) to break such a dependency? Would you be interested in implementing additional JMX beans? You can find some ideas here http://sourceforge.net/tracker/index.php?func=detail&aid=1894138&group_id=209147&atid=1008547 .Things like enabling/disabling tracing thru a JMX Console seem to be useful.

        Show
        Andrew Kornev added a comment - Would you like to verify whether or not there is a Java 6 runtime dependency? Would you be willing to contribute a patch (including this change) to break such a dependency? Would you be interested in implementing additional JMX beans? You can find some ideas here http://sourceforge.net/tracker/index.php?func=detail&aid=1894138&group_id=209147&atid=1008547 .Things like enabling/disabling tracing thru a JMX Console seem to be useful.
        Hide
        Hiram Chirino added a comment -

        Sure.. could we get this patch committed first? I really do fail to see what harm could come from it being committed.

        Show
        Hiram Chirino added a comment - Sure.. could we get this patch committed first? I really do fail to see what harm could come from it being committed.
        Hide
        Andrew Kornev added a comment -

        Committed! Thanks Hiram!

        Show
        Andrew Kornev added a comment - Committed! Thanks Hiram!
        Hide
        Mahadev konar added a comment -

        assigning it to hiram ..

        Show
        Mahadev konar added a comment - assigning it to hiram ..
        Hide
        Patrick Hunt added a comment -

        Fixed in 3.0.0

        Show
        Patrick Hunt added a comment - Fixed in 3.0.0

          People

          • Assignee:
            Hiram Chirino
            Reporter:
            Hiram Chirino
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development