Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: 2.1.0-beta, 3.0.0
    • 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.3.patch
        1 kB
        Akira AJISAKA
      3. HADOOP-9913.2.patch
        1 kB
        Akira AJISAKA

        Issue Links

          Activity

          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.
          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
          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
          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
          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.
          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?
          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
          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 @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
          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.
          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.
          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
          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
          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.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development