Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: 3.0.0, 2.1.0-beta
    • Fix Version/s: None
    • Component/s: documentation, metrics
    • Labels:
    • Environment:

      trunk

      Description

      In o.a.h.ipc.metrics.RpcMetrics.java, metrics are declared as follows:

         @Metric("Queue time") MutableRate rpcQueueTime;
         @Metric("Processsing time") MutableRate rpcProcessingTime;
      

      Since some users may confuse which unit (sec or msec) is correct, they should be documented.

      1. HADOOP-9913.patch
        2 kB
        Akira AJISAKA
      2. HADOOP-9913.2.patch
        1 kB
        Akira AJISAKA
      3. HADOOP-9913.3.patch
        1 kB
        Akira AJISAKA

        Issue Links

          Activity

          Akira AJISAKA created issue -
          Akira AJISAKA made changes -
          Field Original Value New Value
          Attachment HADOOP-9913.patch [ 12600480 ]
          Hide
          Akira AJISAKA added a comment -

          I attached a patch for NameNode and RPC metrics.

          Show
          Akira AJISAKA added a comment - I attached a patch for NameNode and RPC metrics.
          Akira AJISAKA made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

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

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

          +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3029//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3029//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/12600480/HADOOP-9913.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +0 tests included . The patch appears to be a documentation patch that doesn't require tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +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/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3029//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3029//console This message is automatically generated.
          Hide
          Tsuyoshi Ozawa added a comment -

          +1, the policy to fix LGTM.
          Two comment:

          1. Can you understand what "Queue time" stands for? If your answer is positive, it's OK. However, IMHO, "Time in server side queue" is more appropriate expression.
          2. The patch against hadoop-common-project and hadoop-hdfs-project can be split.

          Show
          Tsuyoshi Ozawa added a comment - +1, the policy to fix LGTM. Two comment: 1. Can you understand what "Queue time" stands for? If your answer is positive, it's OK. However, IMHO, "Time in server side queue" is more appropriate expression. 2. The patch against hadoop-common-project and hadoop-hdfs-project can be split.
          Hide
          Akira AJISAKA added a comment -

          Tsuyoshi Ozawa, thanks for your comment.

          Can you understand what "Queue time" stands for? If your answer is positive, it's OK. However, IMHO, "Time in server side queue" is more appropriate expression.

          I understand. I'll fix the patch to make more appropriate expression.

          The patch against hadoop-common-project and hadoop-hdfs-project can be split.

          Thanks. I'll split the patch and make another issue for NameNodeMetrics on HDFS project.

          Show
          Akira AJISAKA added a comment - Tsuyoshi Ozawa , thanks for your comment. Can you understand what "Queue time" stands for? If your answer is positive, it's OK. However, IMHO, "Time in server side queue" is more appropriate expression. I understand. I'll fix the patch to make more appropriate expression. The patch against hadoop-common-project and hadoop-hdfs-project can be split. Thanks. I'll split the patch and make another issue for NameNodeMetrics on HDFS project.
          Akira AJISAKA made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Akira AJISAKA made changes -
          Attachment HADOOP-9913.2.patch [ 12600696 ]
          Akira AJISAKA made changes -
          Summary Document time unit to metrics Document time unit to RpcMetrics
          Akira AJISAKA made changes -
          Summary Document time unit to RpcMetrics Document time unit to RpcMetrics.java
          Akira AJISAKA made changes -
          Description For example, in o.a.h.hdfs.server.namenode.metrics.NameNodeMetrics.java, metrics are declared as follows:

          {code}
            @Metric("Duration in SafeMode at startup") MutableGaugeInt safeModeTime;
            @Metric("Time loading FS Image at startup") MutableGaugeInt fsImageLoadTime;
          {code}

          Since some users may confuse which unit (sec or msec) is correct, they should be documented.
          In o.a.h.hdfs.server.namenode.metrics.NameNodeMetrics.java, metrics are declared as follows:

          {code}
            @Metric("Duration in SafeMode at startup") MutableGaugeInt safeModeTime;
            @Metric("Time loading FS Image at startup") MutableGaugeInt fsImageLoadTime;
          {code}

          Since some users may confuse which unit (sec or msec) is correct, they should be documented.
          Akira AJISAKA made changes -
          Description In o.a.h.hdfs.server.namenode.metrics.NameNodeMetrics.java, metrics are declared as follows:

          {code}
            @Metric("Duration in SafeMode at startup") MutableGaugeInt safeModeTime;
            @Metric("Time loading FS Image at startup") MutableGaugeInt fsImageLoadTime;
          {code}

          Since some users may confuse which unit (sec or msec) is correct, they should be documented.
          In o.a.h.ipc.metrics.RpcMetrics.java, metrics are declared as follows:

          {code}
             @Metric("Queue time") MutableRate rpcQueueTime;
             @Metric("Processsing time") MutableRate rpcProcessingTime;
          {code}

          Since some users may confuse which unit (sec or msec) is correct, they should be documented.
          Hide
          Akira AJISAKA added a comment -

          I created HDFS-5144 for NameNodeMetrics.java.

          Show
          Akira AJISAKA added a comment - I created HDFS-5144 for NameNodeMetrics.java.
          Akira AJISAKA made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Akira AJISAKA made changes -
          Link This issue relates to HDFS-5144 [ HDFS-5144 ]
          Hide
          Hadoop QA added a comment -

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

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

          +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

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

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

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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/hadoop-common.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3041//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3041//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/12600696/HADOOP-9913.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +0 tests included . The patch appears to be a documentation patch that doesn't require tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +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/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3041//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3041//console This message is automatically generated.
          Hide
          Allen Wittenauer added a comment -

          Looks like this patch has gone stale.

          Show
          Allen Wittenauer added a comment - Looks like this patch has gone stale.
          Hide
          Hadoop QA added a comment -

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4474//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/12600696/HADOOP-9913.2.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4474//console This message is automatically generated.
          Hide
          Tsuyoshi Ozawa added a comment -

          Akira AJISAKA, can you refresh a patch?

          Show
          Tsuyoshi Ozawa added a comment - Akira AJISAKA , can you refresh a patch?
          Allen Wittenauer made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Akira AJISAKA added a comment -

          Thanks Allen Wittenauer and Tsuyoshi Ozawa for the comments. Rebased the patch.

          Show
          Akira AJISAKA added a comment - Thanks Allen Wittenauer and Tsuyoshi Ozawa for the comments. Rebased the patch.
          Akira AJISAKA made changes -
          Attachment HADOOP-9913.3.patch [ 12662433 ]
          Akira AJISAKA made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Target Version/s 3.0.0 [ 12320357 ] 3.0.0, 2.6.0 [ 12320357, 12327179 ]
          Hide
          Hadoop QA added a comment -

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

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

          +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 in hadoop-common-project/hadoop-common:

          org.apache.hadoop.ipc.TestDecayRpcScheduler

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4496//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4496//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/12662433/HADOOP-9913.3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +0 tests included . The patch appears to be a documentation patch that doesn't require tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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 in hadoop-common-project/hadoop-common: org.apache.hadoop.ipc.TestDecayRpcScheduler +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4496//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4496//console This message is automatically generated.
          Hide
          Allen Wittenauer added a comment -

          With a bit more sleep, I have a few thoughts on this that it would be good for someone to validate:

          a) Wouldn't this be better as documentation?

          b) Doesn't changing the metric like this actually 'change the metric' that is collected as well? i.e., this is an incompatible change?

          Show
          Allen Wittenauer added a comment - With a bit more sleep, I have a few thoughts on this that it would be good for someone to validate: a) Wouldn't this be better as documentation? b) Doesn't changing the metric like this actually 'change the metric' that is collected as well? i.e., this is an incompatible change?
          Hide
          Akira AJISAKA added a comment -

          Wouldn't this be better as documentation?

          Now we have the documentation which describes the time unit of the metrics, so the priority is very low.

          this is an incompatible change?

          I don't think this is an incompatible change. Compatibility doc says

          Modifying (eg changing the unit or measurement) or removing existing metrics breaks compatibility. Similarly, changes to JMX MBean object names also break compatibility.

          The patch does not modify the unit, the measurement, or the object names. It only modifies the description.

          Show
          Akira AJISAKA added a comment - Wouldn't this be better as documentation? Now we have the documentation which describes the time unit of the metrics, so the priority is very low. this is an incompatible change? I don't think this is an incompatible change. Compatibility doc says Modifying (eg changing the unit or measurement) or removing existing metrics breaks compatibility. Similarly, changes to JMX MBean object names also break compatibility. The patch does not modify the unit, the measurement, or the object names. It only modifies the description.
          Hide
          Akira AJISAKA added a comment -

          Now we have the documentation which describes the time unit of the metrics, so the priority is very low.

          I'll close this issue as duplicate of HADOOP-6350. Thanks.

          Show
          Akira AJISAKA added a comment - Now we have the documentation which describes the time unit of the metrics, so the priority is very low. I'll close this issue as duplicate of HADOOP-6350 . Thanks.
          Akira AJISAKA made changes -
          Link This issue duplicates HADOOP-6350 [ HADOOP-6350 ]
          Akira AJISAKA made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Assignee Akira AJISAKA [ ajisakaa ]
          Resolution Duplicate [ 3 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          351d 5h 31m 2 Allen Wittenauer 15/Aug/14 04:40
          Open Open Patch Available Patch Available
          3d 2h 56m 3 Akira AJISAKA 18/Aug/14 07:10
          Patch Available Patch Available Resolved Resolved
          1d 1h 20m 1 Akira AJISAKA 19/Aug/14 08:31

            People

            • Assignee:
              Unassigned
              Reporter:
              Akira AJISAKA
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development