HBase
  1. HBase
  2. HBASE-11586

HFile's HDFS op latency sampling code is not used

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.98.4
    • Fix Version/s: 0.99.0, 0.98.5, 2.0.0
    • Component/s: None
    • Labels:
      None

      Description

      HFileReaderV2 calls HFile#offerReadLatency and HFileWriterV2 calls HFile#offerWriteLatency but the samples are never retrieved. There are no callers of HFile#getReadLatenciesNanos, HFile#getWriteLatenciesNanos, and related. The three ArrayBlockingQueues we are using as sample buffers in HFile will fill quickly and are never drained.

      There are also no callers of HFile#getReadTimeMs or HFile#getWriteTimeMs, and related, so we are incrementing a set of AtomicLong counters that will never be read nor reset.

      We are calling System.nanoTime in block read and write paths twice but not utilizing the measurements.

      We should hook this code back up to metrics or remove it.

      We are also not using HFile#getChecksumFailuresCount anywhere but in some unit test code.

      1. HBASE-11586.patch
        6 kB
        Andrew Purtell
      2. HBASE-11586.patch
        35 kB
        Andrew Purtell

        Activity

        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Hide
        hongyu bi added a comment -

        we just upgrade 0.98.6 from 0.94.15 , and I can't find the fs

        {read|write}

        ops/latency metrics, what about the new jira? many thanks

        Show
        hongyu bi added a comment - we just upgrade 0.98.6 from 0.94.15 , and I can't find the fs {read|write} ops/latency metrics, what about the new jira? many thanks
        Hide
        Andrew Purtell added a comment -

        Let me fill in the story here Andrew. Will be back.

        Cool, we can put the metric back in a new JIRA I suppose.

        Show
        Andrew Purtell added a comment - Let me fill in the story here Andrew. Will be back. Cool, we can put the metric back in a new JIRA I suppose.
        Hide
        stack added a comment -

        I assume the removal of the metric was an intended and reviewed change though.

        Let me fill in the story here Andrew. Will be back.

        Show
        stack added a comment - I assume the removal of the metric was an intended and reviewed change though. Let me fill in the story here Andrew. Will be back.
        Hide
        Andrew Purtell added a comment -

        Sure, we can revert the current committed change and add back the regionserver metrics for this removed earlier. I assume the removal of the metric was an intended and reviewed change though.

        Show
        Andrew Purtell added a comment - Sure, we can revert the current committed change and add back the regionserver metrics for this removed earlier. I assume the removal of the metric was an intended and reviewed change though.
        Hide
        stack added a comment -

        Seem like useful metrics. How hard to hook them up? I could do it in new issue?

        Show
        stack added a comment - Seem like useful metrics. How hard to hook them up? I could do it in new issue?
        Hide
        Andrew Purtell added a comment -

        Are these the global fs{read|write}Latency histograms that are shown on the region server UI and via JMX

        Yes. The same measurement is also used when updating one of the dynamic schema metrics.

        Show
        Andrew Purtell added a comment - Are these the global fs{read|write}Latency histograms that are shown on the region server UI and via JMX Yes. The same measurement is also used when updating one of the dynamic schema metrics.
        Hide
        Lars Hofhansl added a comment -

        Honestly I am confused where we use/display these metrics. Are these the global fs

        {read|write}

        Latency histograms that are shown on the region server UI and via JMX, or something else altogether. I'll check the 0.94 in a bit.

        Show
        Lars Hofhansl added a comment - Honestly I am confused where we use/display these metrics. Are these the global fs {read|write} Latency histograms that are shown on the region server UI and via JMX, or something else altogether. I'll check the 0.94 in a bit.
        Hide
        Andrew Purtell added a comment -

        Do we need the dynamic schema metrics that these block read and write path measurements feed? If not we could open a 0.94 specific JIRA to remove them. Then this change can be backported. We could do the work with two commits like that perhaps.

        Show
        Andrew Purtell added a comment - Do we need the dynamic schema metrics that these block read and write path measurements feed? If not we could open a 0.94 specific JIRA to remove them. Then this change can be backported. We could do the work with two commits like that perhaps.
        Hide
        Lars Hofhansl added a comment -

        Sorry I change the assignee to me... That was unintended.
        Lemme check the 0.94 code. Every bit of memory barriers we can remove is a win.

        Show
        Lars Hofhansl added a comment - Sorry I change the assignee to me... That was unintended. Lemme check the 0.94 code. Every bit of memory barriers we can remove is a win.
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-TRUNK #5342 (See https://builds.apache.org/job/HBase-TRUNK/5342/)
        HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 531eee003182647e9f944a5cbcb6117555c39e44)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5342 (See https://builds.apache.org/job/HBase-TRUNK/5342/ ) HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 531eee003182647e9f944a5cbcb6117555c39e44) hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-1.0 #70 (See https://builds.apache.org/job/HBase-1.0/70/)
        HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 13643807adffd7f5a798251594f275bc318d00eb)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-1.0 #70 (See https://builds.apache.org/job/HBase-1.0/70/ ) HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 13643807adffd7f5a798251594f275bc318d00eb) hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #398 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/398/)
        HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 27eef5f73960944f6cbaa3894fd58d5e5b3bfc28)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #398 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/398/ ) HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 27eef5f73960944f6cbaa3894fd58d5e5b3bfc28) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #419 (See https://builds.apache.org/job/HBase-0.98/419/)
        HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 27eef5f73960944f6cbaa3894fd58d5e5b3bfc28)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #419 (See https://builds.apache.org/job/HBase-0.98/419/ ) HBASE-11586 HFile's HDFS op latency sampling code is not used (apurtell: rev 27eef5f73960944f6cbaa3894fd58d5e5b3bfc28) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
        Hide
        Andrew Purtell added a comment -

        Committed to 0.98+.

        Let me know if you'd like these removed from 0.94 Lars Hofhansl. We can do that on a new issue.

        Show
        Andrew Purtell added a comment - Committed to 0.98+. Let me know if you'd like these removed from 0.94 Lars Hofhansl . We can do that on a new issue.
        Hide
        Andrew Purtell added a comment -

        We still use the HDFS op latency measurements in 0.94 HFile, for regionserver and dynamic schema metrics.

        Show
        Andrew Purtell added a comment - We still use the HDFS op latency measurements in 0.94 HFile, for regionserver and dynamic schema metrics.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12657725/HBASE-11586.patch
        against trunk revision .
        ATTACHMENT ID: 12657725

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

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

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

        +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 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 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.TestRegionRebalancing

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//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/12657725/HBASE-11586.patch against trunk revision . ATTACHMENT ID: 12657725 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +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 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 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.TestRegionRebalancing Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/10183//console This message is automatically generated.
        Hide
        Lars Hofhansl added a comment -

        Nice. I'll check 0.94 as well.

        Show
        Lars Hofhansl added a comment - Nice. I'll check 0.94 as well.
        Hide
        Andrew Purtell added a comment - - edited

        Updated patch. I was looking this over for commit and noticed that although HFileReadWriteTest wants to use the op count and nanotime accumulators to bench operations, those counters are not updated anywhere. grepped over all modules to confirm. The patch is larger because it now removes HFileReadWriteTest. Change is a continuation of earlier acked work without surprises. Going to commit to 0.98+ in a little while unless objection.

        Show
        Andrew Purtell added a comment - - edited Updated patch. I was looking this over for commit and noticed that although HFileReadWriteTest wants to use the op count and nanotime accumulators to bench operations, those counters are not updated anywhere. grepped over all modules to confirm. The patch is larger because it now removes HFileReadWriteTest. Change is a continuation of earlier acked work without surprises. Going to commit to 0.98+ in a little while unless objection.
        Hide
        Enis Soztutar added a comment - - edited

        +1 for branch-1+.

        Show
        Enis Soztutar added a comment - - edited +1 for branch-1+.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12657668/HBASE-11586.patch
        against trunk revision .
        ATTACHMENT ID: 12657668

        +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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +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 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 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

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

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//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/12657668/HBASE-11586.patch against trunk revision . ATTACHMENT ID: 12657668 +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 javac . The applied patch does not increase the total number of javac compiler warnings. +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 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 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/10180//console This message is automatically generated.
        Hide
        Srikanth Srungarapu added a comment -

        +1 (non-binding vote).

        Show
        Srikanth Srungarapu added a comment - +1 (non-binding vote).
        Hide
        Andrew Purtell added a comment -

        Attached patch removes the sample buffers, related methods, and unused measuring in the HFile reader and writer.

        The atomic variables for op count and nanotime accumulators are still used by HFileReadWriteTest, in a test package of hbase-server.

        Show
        Andrew Purtell added a comment - Attached patch removes the sample buffers, related methods, and unused measuring in the HFile reader and writer. The atomic variables for op count and nanotime accumulators are still used by HFileReadWriteTest, in a test package of hbase-server.

          People

          • Assignee:
            Andrew Purtell
            Reporter:
            Andrew Purtell
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development