HBase
  1. HBase
  2. HBASE-10805

Speed up KeyValueHeap.next() a bit

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.98.1, 0.99.0, 0.94.19, 0.96.3
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      See discussion on HBASE-9969.
      This was brought up by Matt Corgan. Even though I didn't believe him first.

            KeyValueScanner topScanner = this.heap.peek();
            if (topScanner == null ||
                this.comparator.compare(kvNext, topScanner.peek()) >= 0) {
              this.heap.add(this.current);
              this.current = pollRealKV();
            }
      

      We already have the invariant everywhere this.current always has a real KV polled. So adding current back to the heap followed by pollRealKV() is a no-op if this.current is the only scanner anyway.

      This can be changed to:

            ...
            if (topScanner != null &&
                this.comparator.compare(kvNext, topScanner.peek()) >= 0) {
            ...
      

      With 20m rows, 5 cols each, everything in the cache, fully compacted - i.e. one HFile per store, a scan that filters everything at the server through all 100m KVs takes 15.1s without the change and 13.3s with it, so a 12% improvement.

      1. 10805-trunk.txt
        0.8 kB
        Lars Hofhansl

        Activity

        Lars Hofhansl made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Andrew Purtell made changes -
        Fix Version/s 0.98.1 [ 12325664 ]
        Fix Version/s 0.98.2 [ 12326505 ]
        Hide
        Hudson added a comment -

        FAILURE: Integrated in hbase-0.96-hadoop2 #249 (See https://builds.apache.org/job/hbase-0.96-hadoop2/249/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580147)

        • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in hbase-0.96-hadoop2 #249 (See https://builds.apache.org/job/hbase-0.96-hadoop2/249/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580147) /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in hbase-0.96 #364 (See https://builds.apache.org/job/hbase-0.96/364/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580147)

        • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in hbase-0.96 #364 (See https://builds.apache.org/job/hbase-0.96/364/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580147) /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-TRUNK #5033 (See https://builds.apache.org/job/HBase-TRUNK/5033/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580149)

        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #5033 (See https://builds.apache.org/job/HBase-TRUNK/5033/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580149) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #247 (See https://builds.apache.org/job/HBase-0.98/247/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580148)

        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #247 (See https://builds.apache.org/job/HBase-0.98/247/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580148) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #231 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/231/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580148)

        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #231 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/231/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580148) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94 #1332 (See https://builds.apache.org/job/HBase-0.94/1332/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580146)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94 #1332 (See https://builds.apache.org/job/HBase-0.94/1332/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580146) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #60 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/60/)
        HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580146)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #60 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/60/ ) HBASE-10805 Speed up KeyValueHeap.next() a bit (larsh: rev 1580146) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
        Lars Hofhansl made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Assignee Lars Hofhansl [ lhofhansl ]
        Resolution Fixed [ 1 ]
        Hide
        Lars Hofhansl added a comment -

        Committed to all branches. Thanks for taking a look.

        Show
        Lars Hofhansl added a comment - Committed to all branches. Thanks for taking a look.
        Hide
        stack added a comment -

        +1 for 0.96

        Show
        stack added a comment - +1 for 0.96
        Hide
        Andrew Purtell added a comment -

        +1 from me. We can back it out if tests go flaky subsequently. (I'm about to give 0.98 branch another round of 50 suite runs.)

        Show
        Andrew Purtell added a comment - +1 from me. We can back it out if tests go flaky subsequently. (I'm about to give 0.98 branch another round of 50 suite runs.)
        Hide
        Lars Hofhansl added a comment -

        I reran TestZKProcedure, TestEncryptionKeyRotation, and TestLogRolling locally. The all pass fine (and in any case they do not seem related).
        So if no objection I would like to commit this to all branches. Andrew Purtell, stack.

        Show
        Lars Hofhansl added a comment - I reran TestZKProcedure, TestEncryptionKeyRotation, and TestLogRolling locally. The all pass fine (and in any case they do not seem related). So if no objection I would like to commit this to all branches. Andrew Purtell , stack .
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12635957/10805-trunk.txt
        against trunk revision .
        ATTACHMENT ID: 12635957

        +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 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.procedure.TestZKProcedure

        -1 core zombie tests. There are 2 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestEncryptionKeyRotation.testCFKeyRotation(TestEncryptionKeyRotation.java:101)
        at org.apache.hadoop.hbase.regionserver.wal.TestLogRolling.testLogRollOnDatanodeDeath(TestLogRolling.java:368)

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//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/12635957/10805-trunk.txt against trunk revision . ATTACHMENT ID: 12635957 +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 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.procedure.TestZKProcedure -1 core zombie tests . There are 2 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestEncryptionKeyRotation.testCFKeyRotation(TestEncryptionKeyRotation.java:101) at org.apache.hadoop.hbase.regionserver.wal.TestLogRolling.testLogRollOnDatanodeDeath(TestLogRolling.java:368) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9058//console This message is automatically generated.
        Hide
        stack added a comment -

        nice

        Show
        stack added a comment - nice
        Hide
        Lars Hofhansl added a comment -

        Also tried with 20m rows, 1 col each, every in cache and filtered at the server. Without patch takes 5.63s, with 5.19s.

        Show
        Lars Hofhansl added a comment - Also tried with 20m rows, 1 col each, every in cache and filtered at the server. Without patch takes 5.63s, with 5.19s.
        Lars Hofhansl made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Lars Hofhansl made changes -
        Field Original Value New Value
        Attachment 10805-trunk.txt [ 12635957 ]
        Hide
        Lars Hofhansl added a comment -

        Trunk patch to get a test run through, just to be sure.

        Show
        Lars Hofhansl added a comment - Trunk patch to get a test run through, just to be sure.
        Lars Hofhansl created issue -

          People

          • Assignee:
            Lars Hofhansl
            Reporter:
            Lars Hofhansl
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development