Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.20.2
    • Fix Version/s: 0.23.0, 0.24.0
    • Component/s: metrics
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      JVM metrics published to Ganglia now include the process name as part of the gmetric name.

      Description

      Ganglia jvm metrics don't make sense because it's not clear which java process the metrics refer to. In fact, all hadoop java processes running on a node report their jvm metrics to the same namespace.

      The metrics are exposed by the "jvm" context in JvmMetrics.java. This leads to confusing and nonsensical graphs in ganglia and maybe other monitoring tools.

      One way to fix this is to make sure the process name is reported in the jvm context, making it clear which process is associated with the context, and separating out the jvm metrics per process.

      This is marked as an "incompatible change" because the fix provided removes the JVM metrics and replaces it with process-specific metrics.

      1. JvmMetrics.java
        7 kB
        Jeff Bean
      2. hadoop-metrics.properties
        2 kB
        Jeff Bean
      3. HADOOP-7507v7.patch
        9 kB
        Alejandro Abdelnur
      4. HADOOP-7507v6.patch
        9 kB
        Alejandro Abdelnur
      5. HADOOP-7507v5.patch
        3 kB
        Alejandro Abdelnur
      6. HADOOP-7507v4.patch
        3 kB
        Alejandro Abdelnur
      7. HADOOP-7507v3.patch
        2 kB
        Alejandro Abdelnur
      8. HADOOP-7507-v2.patch
        2 kB
        Michael Katzenellenbogen
      9. HADOOP-7507v1.patch
        2 kB
        Alejandro Abdelnur
      10. ASF.LICENSE.NOT.GRANTED--screenshot-1.jpg
        167 kB
        Jeff Bean

        Activity

        Jeff Bean created issue -
        Hide
        Jeff Bean added a comment -

        Proposed fix. - a one line change to JvmMetrics.java and a sample hadoop-metrics.properties which includes process-specific metrics.

        Show
        Jeff Bean added a comment - Proposed fix. - a one line change to JvmMetrics.java and a sample hadoop-metrics.properties which includes process-specific metrics.
        Jeff Bean made changes -
        Field Original Value New Value
        Attachment hadoop-metrics.properties [ 12489218 ]
        Attachment JvmMetrics.java [ 12489219 ]
        Hide
        Jeff Bean added a comment -

        This screen shot shows how process-specific jvm metrics can appear in ganglia.

        Show
        Jeff Bean added a comment - This screen shot shows how process-specific jvm metrics can appear in ganglia.
        Jeff Bean made changes -
        Attachment screenshot-1.jpg [ 12489220 ]
        Alejandro Abdelnur made changes -
        Assignee Alejandro Abdelnur [ tucu00 ]
        Hide
        Alejandro Abdelnur added a comment -

        Patch based on Jeff's changes (thanks Jeff)

        Show
        Alejandro Abdelnur added a comment - Patch based on Jeff's changes (thanks Jeff)
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v1.patch [ 12490307 ]
        Alejandro Abdelnur made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Luke Lu added a comment -

        This is actually not the right fix as jvm metrics already set the processName tag. The right place to fix this is actually in ganglia plugin to encode tags as part of the metrics names, as the problem not only happens to jvm metrics but also other per tag metrics as well, say per queue metrics in capacity scheduler.

        Both metrics1 and metrics2 Ganglia

        {Plugin,Sink}

        needs to be fixed. Metrics2 is what hadoop will be using for trunk/0.23. Some downstream projects like HBase will still use metrics1 for compatibility reasons (working with multiple hadoop releases from 0.20 to 0.23 etc.)

        Show
        Luke Lu added a comment - This is actually not the right fix as jvm metrics already set the processName tag. The right place to fix this is actually in ganglia plugin to encode tags as part of the metrics names, as the problem not only happens to jvm metrics but also other per tag metrics as well, say per queue metrics in capacity scheduler. Both metrics1 and metrics2 Ganglia {Plugin,Sink} needs to be fixed. Metrics2 is what hadoop will be using for trunk/0.23. Some downstream projects like HBase will still use metrics1 for compatibility reasons (working with multiple hadoop releases from 0.20 to 0.23 etc.)
        Hide
        Michael Katzenellenbogen added a comment -

        Ran into this issue as well. Haven't fully tested the patch yet (functionally), but uploading for reference.

        Show
        Michael Katzenellenbogen added a comment - Ran into this issue as well. Haven't fully tested the patch yet (functionally), but uploading for reference.
        Michael Katzenellenbogen made changes -
        Attachment HADOOP-7507-v2.patch [ 12490506 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12490506/HADOOP-7507-v2.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        +1 system test framework. The patch passed system test framework compile.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//artifact/trunk/target/newPatchFindbugsWarningshadoop-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//artifact/trunk/target/newPatchFindbugsWarningshadoop-annotations.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12490506/HADOOP-7507-v2.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//artifact/trunk/target/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//artifact/trunk/target/newPatchFindbugsWarningshadoop-annotations.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/40//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        +1.

        I've applied 'v2' patch and manually tested with the different versions of ganglia, names are now namespaced. I.e.:

        <METRIC NAME="jvm.NameNode.metrics.logWarn" VAL="0" TYPE="int32" UNITS="" TN="5" TMAX="60" DMAX="0" SLOPE="both">
        
        Show
        Alejandro Abdelnur added a comment - +1. I've applied 'v2' patch and manually tested with the different versions of ganglia, names are now namespaced. I.e.: <METRIC NAME= "jvm.NameNode.metrics.logWarn" VAL= "0" TYPE= "int32" UNITS= "" TN=" 5 " TMAX=" 60 " DMAX=" 0 " SLOPE=" both">
        Hide
        Alejandro Abdelnur added a comment -

        patch has to be rebased to new layout. And there is concatenation typo.

        Show
        Alejandro Abdelnur added a comment - patch has to be rebased to new layout. And there is concatenation typo.
        Alejandro Abdelnur made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v3.patch [ 12491713 ]
        Hide
        Alejandro Abdelnur added a comment -

        rebased and concat typo fixed

        Show
        Alejandro Abdelnur added a comment - rebased and concat typo fixed
        Alejandro Abdelnur made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Fix Version/s 0.23.0 [ 12315569 ]
        Hide
        Todd Lipcon added a comment -

        +1 pending Hudson

        Show
        Todd Lipcon added a comment - +1 pending Hudson
        Hide
        Luke Lu added a comment -

        The patch only works around the jvm metrics problem. It doesn't really fix the underlying issue of the Ganglia support: lack of tag/dimension/group support (They are working on tags support). Our current workaround is flatten the namespace, but in a inconsistent way, so the it still won't work for QueueMetrics in CapacityScheduler.

        I think for metrics1, the hardcoded workaround is probably OK (to minimize disruption to old systems) but for metrics2, we could do something like this for the groupName:

        for (MetricsTag t : tags) {
          if (t.value() != null) {
            sb.append('.').append(t.value());
          }
        }
        
        Show
        Luke Lu added a comment - The patch only works around the jvm metrics problem. It doesn't really fix the underlying issue of the Ganglia support: lack of tag/dimension/group support (They are working on tags support ). Our current workaround is flatten the namespace, but in a inconsistent way, so the it still won't work for QueueMetrics in CapacityScheduler. I think for metrics1, the hardcoded workaround is probably OK (to minimize disruption to old systems) but for metrics2, we could do something like this for the groupName: for (MetricsTag t : tags) { if (t.value() != null ) { sb.append('.').append(t.value()); } }
        Hide
        Luke Lu added a comment -

        Probably should exclude hostname, as ganglia handles it explicitly:

        for (MetricsTag t : tags) {
          if (t.info() != MsInfo.Hostname && t.value() != null) {
            sb.append('.').append(t.value());
          }
        }
        
        Show
        Luke Lu added a comment - Probably should exclude hostname, as ganglia handles it explicitly: for (MetricsTag t : tags) { if (t.info() != MsInfo.Hostname && t.value() != null ) { sb.append('.').append(t.value()); } }
        Hide
        Todd Lipcon added a comment -

        We'd also want a consistent ordering of tags, right? (perhaps sorted by their keys)

        Show
        Todd Lipcon added a comment - We'd also want a consistent ordering of tags, right? (perhaps sorted by their keys)
        Hide
        Luke Lu added a comment -

        The ordering of the tags is determined by the metrics sources (typically managed by MetricsRegistry, which uses a LinkedHashMap to manage tags). The same metrics record with different tags are from different instances of the same metrics classes. So sorting is not necessary. I personally would prefer being able control the ordering.

        Show
        Luke Lu added a comment - The ordering of the tags is determined by the metrics sources (typically managed by MetricsRegistry, which uses a LinkedHashMap to manage tags). The same metrics record with different tags are from different instances of the same metrics classes. So sorting is not necessary. I personally would prefer being able control the ordering.
        Hide
        Alejandro Abdelnur added a comment -

        v4 patch.

        Integrates Luke's suggestions.

        I've checked with Matt regarding the exclusion of the Hostname tag, and he indicated that it is a good idea. His argument is that hostnames are case insensitive and different DNSes may have the host with different case and this could create shadow groups if a process starts getting a different case DNS name.

        Show
        Alejandro Abdelnur added a comment - v4 patch. Integrates Luke's suggestions. I've checked with Matt regarding the exclusion of the Hostname tag, and he indicated that it is a good idea. His argument is that hostnames are case insensitive and different DNSes may have the host with different case and this could create shadow groups if a process starts getting a different case DNS name.
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v4.patch [ 12492109 ]
        Hide
        Luke Lu added a comment -

        v4 patch integrated generic tags handling in jvm context only i.e., the outer if check for jvm context is unnecessary.

        Show
        Luke Lu added a comment - v4 patch integrated generic tags handling in jvm context only i.e., the outer if check for jvm context is unnecessary.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12492109/HADOOP-7507v4.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12492109/HADOOP-7507v4.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 4 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/96//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        v5 patch addressing last comment from Luke.

        Show
        Alejandro Abdelnur added a comment - v5 patch addressing last comment from Luke.
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v5.patch [ 12492641 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12492641/HADOOP-7507v5.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:

        org.apache.hadoop.io.serializer.avro.TestAvroSerialization
        org.apache.hadoop.io.TestText
        org.apache.hadoop.conf.TestConfiguration
        org.apache.hadoop.io.TestEnumSetWritable
        org.apache.hadoop.fs.TestPath
        org.apache.hadoop.ipc.TestAvroRpc

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12492641/HADOOP-7507v5.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.io.serializer.avro.TestAvroSerialization org.apache.hadoop.io.TestText org.apache.hadoop.conf.TestConfiguration org.apache.hadoop.io.TestEnumSetWritable org.apache.hadoop.fs.TestPath org.apache.hadoop.ipc.TestAvroRpc +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/121//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        Thinking about this a bit... it seems like doing the generic tag handling won't work well in Ganglia, given that the JT has job-specific metrics tagged by the job ID (eg fairscheduler job level metrics). Over time, the JT will accumulate an unbounded number of metrics in ganglia, and I don't think Ganglia ever "ages off" a gmetric that it hasn't seen in a while. I think we need some sort of config for the ganglia context that says which tag names need to be included as part of a metric (and perhaps which metrics shouldn't be reported at all in ganglia?)

        Show
        Todd Lipcon added a comment - Thinking about this a bit... it seems like doing the generic tag handling won't work well in Ganglia, given that the JT has job-specific metrics tagged by the job ID (eg fairscheduler job level metrics). Over time, the JT will accumulate an unbounded number of metrics in ganglia, and I don't think Ganglia ever "ages off" a gmetric that it hasn't seen in a while. I think we need some sort of config for the ganglia context that says which tag names need to be included as part of a metric (and perhaps which metrics shouldn't be reported at all in ganglia?)
        Hide
        Alejandro Abdelnur added a comment -

        there is something wrong in the logic, checking.

        Show
        Alejandro Abdelnur added a comment - there is something wrong in the logic, checking.
        Alejandro Abdelnur made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Luke Lu added a comment -

        generic tag handling won't work well in Ganglia, given that the JT has job-specific metrics tagged by the job ID (eg fairscheduler job level metrics)

        Ganglia cannot handle this type of metrics correctly anyway. metrics2 can be configured (dynamically) to filter out certain portion of metrics (by context/source/metrics/tag with glob pattern and regex) per metrics sink.

        Show
        Luke Lu added a comment - generic tag handling won't work well in Ganglia, given that the JT has job-specific metrics tagged by the job ID (eg fairscheduler job level metrics) Ganglia cannot handle this type of metrics correctly anyway. metrics2 can be configured (dynamically) to filter out certain portion of metrics (by context/source/metrics/tag with glob pattern and regex) per metrics sink.
        Hide
        Luke Lu added a comment -

        there is something wrong in the logic, checking.

        ProcessName checking and the break is unnecessary. The whole block of logic should look like the code in this comment.

        Show
        Luke Lu added a comment - there is something wrong in the logic, checking. ProcessName checking and the break is unnecessary. The whole block of logic should look like the code in this comment .
        Hide
        Alejandro Abdelnur added a comment -

        v6,

        For 'metrics' the original fix stays, just prefixing the processName to the 'jvm' context.

        For 'metrics2' rewritten (as per Todd's suggestion) to be able to specify which tags are to be used for the prefix.

        In the hadoop-metrics2.properties the property PROCESS_NAME.sink.ganglia.tagsForPrefix.CONTEXT_NAME indicates which tags are used. If not specified, no tag is used, if '*' is specified all tags are used, individual tags can be specified separated by commas.

        For example:

        namenode.sink.ganglia.tagsForPrefix.jvm=*
        namenode.sink.ganglia.tagsForPrefix.dfs=*
        

        The Context and Hostname tags are never used.

        Because the default is no tags, the default behavior is backwards compatible.

        Show
        Alejandro Abdelnur added a comment - v6, For 'metrics' the original fix stays, just prefixing the processName to the 'jvm' context. For 'metrics2' rewritten (as per Todd's suggestion) to be able to specify which tags are to be used for the prefix. In the hadoop-metrics2.properties the property PROCESS_NAME.sink.ganglia.tagsForPrefix.CONTEXT_NAME indicates which tags are used. If not specified, no tag is used, if '*' is specified all tags are used, individual tags can be specified separated by commas. For example: namenode.sink.ganglia.tagsForPrefix.jvm=* namenode.sink.ganglia.tagsForPrefix.dfs=* The Context and Hostname tags are never used. Because the default is no tags, the default behavior is backwards compatible.
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v6.patch [ 12492679 ]
        Alejandro Abdelnur made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Alejandro Abdelnur made changes -
        Fix Version/s 0.24.0 [ 12317652 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12492679/HADOOP-7507v6.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:

        org.apache.hadoop.io.serializer.avro.TestAvroSerialization
        org.apache.hadoop.io.TestText
        org.apache.hadoop.conf.TestConfiguration
        org.apache.hadoop.io.TestEnumSetWritable
        org.apache.hadoop.fs.TestPath
        org.apache.hadoop.ipc.TestAvroRpc

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12492679/HADOOP-7507v6.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.io.serializer.avro.TestAvroSerialization org.apache.hadoop.io.TestText org.apache.hadoop.conf.TestConfiguration org.apache.hadoop.io.TestEnumSetWritable org.apache.hadoop.fs.TestPath org.apache.hadoop.ipc.TestAvroRpc +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/127//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -
        • In hadoop-metrics2.properties, we should probably default tagsForPrefix.jvm=ProcessName, right? So that the default behavior with the new metrics system solves the original issue of this JIRA
        • TAGS_FOR_PREFIX_PROPERTY can be private
        • minor efficiency nit: change createPrefix() to be appendPrefix(MetricsRecord record, StringBuilder builder) so you can just call appendPrefix(record, sb);
        • createPrefix could be package-private, right?
        Show
        Todd Lipcon added a comment - In hadoop-metrics2.properties, we should probably default tagsForPrefix.jvm=ProcessName, right? So that the default behavior with the new metrics system solves the original issue of this JIRA TAGS_FOR_PREFIX_PROPERTY can be private minor efficiency nit: change createPrefix() to be appendPrefix(MetricsRecord record, StringBuilder builder) so you can just call appendPrefix(record, sb); createPrefix could be package-private, right?
        Hide
        Alejandro Abdelnur added a comment -

        v7 addresses Todd's comments with a minor twist, using @InterfaceAudience.Private annotation because the testcase is in a different package.

        Show
        Alejandro Abdelnur added a comment - v7 addresses Todd's comments with a minor twist, using @InterfaceAudience.Private annotation because the testcase is in a different package.
        Alejandro Abdelnur made changes -
        Attachment HADOOP-7507v7.patch [ 12493507 ]
        Hide
        Todd Lipcon added a comment -

        I was thinking the default for the jvm prefix would be uncommented in the file. So, if you enable Ganglia metrics, it's set up correctly by default with no other changes. You could uncomment all of the lines, to be consistent, right? the uncommented line with an empty value would be equivalent to the commented-out line?

        For the method being accessible to tests, I think the @VisibleForTests annotation from Guava is more appropriate.

        Show
        Todd Lipcon added a comment - I was thinking the default for the jvm prefix would be uncommented in the file. So, if you enable Ganglia metrics, it's set up correctly by default with no other changes. You could uncomment all of the lines, to be consistent, right? the uncommented line with an empty value would be equivalent to the commented-out line? For the method being accessible to tests, I think the @VisibleForTests annotation from Guava is more appropriate.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12493507/HADOOP-7507v7.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-common-project.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12493507/HADOOP-7507v7.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-common-project. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/148//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        On the annotation, are we using Guava annotations for that in Hadoop?

        Regarding uncommenting the .sink.ganglia.tagsForPrefix.jvm=, that is a good idea, but what are the '' in the new world (MR2)? Can we defer this to a follow up tune-up JIRA?

        Show
        Alejandro Abdelnur added a comment - On the annotation, are we using Guava annotations for that in Hadoop? Regarding uncommenting the .sink.ganglia.tagsForPrefix.jvm=, that is a good idea, but what are the ' ' in the new world (MR2)? Can we defer this to a follow up tune-up JIRA?
        Hide
        Todd Lipcon added a comment -

        On the annotation, are we using Guava annotations for that in Hadoop?

        Not a lot, yet - I've used it a few places in HDFS, but that's about it. I'm fine with leaving it as you have it.

        Can we defer this to a follow up tune-up JIRA?

        Oh, I see what you're saying. Let's defer to a followup once metrics2 starts getting real usage, like you said.

        +1, I'll commit this momentarily

        Show
        Todd Lipcon added a comment - On the annotation, are we using Guava annotations for that in Hadoop? Not a lot, yet - I've used it a few places in HDFS, but that's about it. I'm fine with leaving it as you have it. Can we defer this to a follow up tune-up JIRA? Oh, I see what you're saying. Let's defer to a followup once metrics2 starts getting real usage, like you said. +1, I'll commit this momentarily
        Hide
        Todd Lipcon added a comment -

        Committed to trunk and 0.23. Should we include this in the 0.20 series as well? Since it's a slightly incompatible change, maybe we need to add a config flag to enable the fixed "jvm" metric in metrics1 in the 0.20 backport?

        Show
        Todd Lipcon added a comment - Committed to trunk and 0.23. Should we include this in the 0.20 series as well? Since it's a slightly incompatible change, maybe we need to add a config flag to enable the fixed "jvm" metric in metrics1 in the 0.20 backport?
        Todd Lipcon made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Incompatible change] [Incompatible change, Reviewed]
        Resolution Fixed [ 1 ]
        Todd Lipcon made changes -
        Release Note JVM metrics published to Ganglia now include the process name as part of the gmetric name.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #845 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/845/)
        HADOOP-7507. Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #845 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/845/ ) HADOOP-7507 . Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #922 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/922/)
        HADOOP-7507. Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #922 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/922/ ) HADOOP-7507 . Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #856 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/856/)
        HADOOP-7507. Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #856 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/856/ ) HADOOP-7507 . Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #787 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/787/)
        HADOOP-7507. Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #787 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/787/ ) HADOOP-7507 . Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #810 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/810/)
        HADOOP-7507. Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #810 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/810/ ) HADOOP-7507 . Allow ganglia metrics to include the metrics system tags in the gmetric names. Contributed by Alejandro Abdelnur. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166460 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/conf/hadoop-metrics2.properties /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics/ganglia/GangliaContext.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/sink/ganglia/GangliaSink30.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/impl/TestGangliaMetrics.java
        Arun C Murthy made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Alejandro Abdelnur
            Reporter:
            Jeff Bean
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development