Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Incomplete
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: metrics
    • Labels:
      None

      Description

      We want to evolve the Hadoop metrics subsystem into a more generic Instrumentation facility. The ultimate goal is to add structured logging to Hadoop, with causal tags, a la X-trace. The existing metrics framework is not quite suitable for our needs, since the implementation and interface are tightly coupled. There's no way to use the metrics instrumentation points for anything other than metricss, and there's no way for a metrics context to find out what event just happened.

      We want to tease apart the generic notion of hookable instrumentation points, from the specifics of the information recording. The latter ought to be pluggable at run-time.

      1. instrumentation.patch
        9 kB
        Ari Rabkin
      2. instrumentation-2.patch
        30 kB
        Ari Rabkin
      3. instrumentation-3.patch
        33 kB
        Ari Rabkin
      4. instrumentation-4.patch
        33 kB
        Ari Rabkin
      5. tweak_inst.patch
        0.6 kB
        Ari Rabkin

        Issue Links

          Activity

          Hide
          Ari Rabkin added a comment -

          I think the right way to do this is to add several Instrumentation interfaces, such as JobInstrumentation, NameNodeInstrumentation, DNInstrumentation. These classes would have one method per instrumentation point; and all the metrics computation, causal logging, Chukwa integration, and so forth would be hidden in implementations. Much of Metrics, such as the visible methods of JobTrackerMetrics and much of the MetricsContextFactory, can be reused. Other parts can be encapsulated.

          We'll probably start with TaskTrackerMetrics. We can then build an implementation of TaskTrackerInstrumentation that notifies Chukwa to pick up the task's stdout and stderr. This gives us the hooks to do log and console out harvesting for Chukwa, which lets us close HADOOP-2206, and probably HADOOP-1199.

          Show
          Ari Rabkin added a comment - I think the right way to do this is to add several Instrumentation interfaces, such as JobInstrumentation, NameNodeInstrumentation, DNInstrumentation. These classes would have one method per instrumentation point; and all the metrics computation, causal logging, Chukwa integration, and so forth would be hidden in implementations. Much of Metrics, such as the visible methods of JobTrackerMetrics and much of the MetricsContextFactory, can be reused. Other parts can be encapsulated. We'll probably start with TaskTrackerMetrics. We can then build an implementation of TaskTrackerInstrumentation that notifies Chukwa to pick up the task's stdout and stderr. This gives us the hooks to do log and console out harvesting for Chukwa, which lets us close HADOOP-2206 , and probably HADOOP-1199 .
          Hide
          Ari Rabkin added a comment -

          Here's a patch showing the sort of refactoring I was talking about.

          Unfortunately, the surgery was more involved than I expected: I had forgotten that TaskTracker.TaskTrackerMetrics was an inner class, which had to change.

          Show
          Ari Rabkin added a comment - Here's a patch showing the sort of refactoring I was talking about. Unfortunately, the surgery was more involved than I expected: I had forgotten that TaskTracker.TaskTrackerMetrics was an inner class, which had to change.
          Hide
          Doug Cutting added a comment -

          This looks like a reasonable approach. A few nits:

          It would be cleaner if TaskTracker.Instrumentation were a standalone class. TaskTracker.java is already plenty big! Also, since it is public, this class needs javadoc. The 'tt' field should be private w/ accessor methods or set by the constructor. Should it be abstract? The reference to 'tt.mapTotal' should be replaced with a call to a public method, etc.

          Static accessor methods should be added to TaskTracker to get/set the instrumentation impl, e.g.:

          public static void setInstrumentation(Configuration conf, Class clazz)

          { ... }
          public Class getInstrumentation(Configuration conf) { ... }

          These should refer to a constant that names the configuration parameter name. These methods should be used in preference to using the paramter name literal. An entry should be added to hadoop-default.xml, documenting this new configuration option.

          Show
          Doug Cutting added a comment - This looks like a reasonable approach. A few nits: It would be cleaner if TaskTracker.Instrumentation were a standalone class. TaskTracker.java is already plenty big! Also, since it is public, this class needs javadoc. The 'tt' field should be private w/ accessor methods or set by the constructor. Should it be abstract? The reference to 'tt.mapTotal' should be replaced with a call to a public method, etc. Static accessor methods should be added to TaskTracker to get/set the instrumentation impl, e.g.: public static void setInstrumentation(Configuration conf, Class clazz) { ... } public Class getInstrumentation(Configuration conf) { ... } These should refer to a constant that names the configuration parameter name. These methods should be used in preference to using the paramter name literal. An entry should be added to hadoop-default.xml, documenting this new configuration option.
          Hide
          Ari Rabkin added a comment -

          Corrects nits identified by Doug.

          Note – no test cases included, since it's basically a refactoring, and not a new feature.

          Show
          Ari Rabkin added a comment - Corrects nits identified by Doug. Note – no test cases included, since it's basically a refactoring, and not a new feature.
          Hide
          Ari Rabkin added a comment -

          add JobTracker instrumentation, document options in hadoop-default.xml

          Show
          Ari Rabkin added a comment - add JobTracker instrumentation, document options in hadoop-default.xml
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12386422/instrumentation-2.patch
          against trunk revision 677872.

          +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 tests are needed for 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 1 new Findbugs warnings.

          -1 release audit. The applied patch generated 213 release audit warnings (more than the trunk's current 209 warnings).

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/testReport/
          Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/current/releaseAuditDiffWarnings.txt
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/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/12386422/instrumentation-2.patch against trunk revision 677872. +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 tests are needed for 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 1 new Findbugs warnings. -1 release audit. The applied patch generated 213 release audit warnings (more than the trunk's current 209 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/current/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2904/console This message is automatically generated.
          Hide
          Ari Rabkin added a comment -

          this time, with Apache license at the top.

          Show
          Ari Rabkin added a comment - this time, with Apache license at the top.
          Hide
          Ari Rabkin added a comment -

          fixed findbugs, release warnings.

          Show
          Ari Rabkin added a comment - fixed findbugs, release warnings.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12386436/instrumentation-3.patch
          against trunk revision 678080.

          +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 tests are needed for 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 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.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/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/12386436/instrumentation-3.patch against trunk revision 678080. +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 tests are needed for 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 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. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2909/console This message is automatically generated.
          Hide
          Ari Rabkin added a comment -

          As mentioned above – no test cases included, since this is a refactoring, with no functional changes.

          Show
          Ari Rabkin added a comment - As mentioned above – no test cases included, since this is a refactoring, with no functional changes.
          Hide
          Ari Rabkin added a comment -

          patch was broken by a hadoop update; about to fix.

          Show
          Ari Rabkin added a comment - patch was broken by a hadoop update; about to fix.
          Hide
          Ari Rabkin added a comment -

          revised to match Hadoop trunk

          Show
          Ari Rabkin added a comment - revised to match Hadoop trunk
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12387040/instrumentation-4.patch
          against trunk revision 679930.

          +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 tests are needed for 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 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.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/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/12387040/instrumentation-4.patch against trunk revision 679930. +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 tests are needed for 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 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. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2961/console This message is automatically generated.
          Hide
          Doug Cutting added a comment -

          +1 This looks good to me.

          Show
          Doug Cutting added a comment - +1 This looks good to me.
          Hide
          Arun C Murthy added a comment -

          I just committed this. Thanks, Ari!

          Show
          Arun C Murthy added a comment - I just committed this. Thanks, Ari!
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Seems like errors crept into hadoop-default.xml:

          <property>
          <name>mapred.job.instrumentation</name>
          <value>org.apache.hadoop.mapred.JobTrackerMetricsInst</value>
          <description>Expert: The instrumentation class to associate with each TaskTracker.
          </description>
          </property>

          The property name should be mapred.jobtracker.instrumentation. Also, the description above should be "[...] class to associate with the JobTracker."

          Show
          Vinod Kumar Vavilapalli added a comment - Seems like errors crept into hadoop-default.xml: <property> <name>mapred.job.instrumentation</name> <value>org.apache.hadoop.mapred.JobTrackerMetricsInst</value> <description>Expert: The instrumentation class to associate with each TaskTracker. </description> </property> The property name should be mapred.jobtracker.instrumentation . Also, the description above should be " [...] class to associate with the JobTracker ."
          Hide
          Ari Rabkin added a comment -

          Whups, conf file part of patch was slightly wrong

          Show
          Ari Rabkin added a comment - Whups, conf file part of patch was slightly wrong
          Hide
          Ari Rabkin added a comment -

          Fix conf file mishap. Good catch, Vinod!

          Show
          Ari Rabkin added a comment - Fix conf file mishap. Good catch, Vinod!
          Hide
          Ari Rabkin added a comment -

          Fixes mistake in conf file. No alteration to code, no functional change, no tests.

          Show
          Ari Rabkin added a comment - Fixes mistake in conf file. No alteration to code, no functional change, no tests.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12387481/tweak_inst.patch
          against trunk revision 682978.

          +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 tests are needed for 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 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.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/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/12387481/tweak_inst.patch against trunk revision 682978. +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 tests are needed for 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 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. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3011/console This message is automatically generated.
          Hide
          Hudson added a comment -
          Show
          Hudson added a comment - Integrated in Hadoop-trunk #581 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/581/ )

            People

            • Assignee:
              Ari Rabkin
              Reporter:
              Ari Rabkin
            • Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development