Details

    • Sub-task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 2.0.0
    • regionserver, Scanners
    • None
    • Reviewed

    Description

      Once we are changing the read path to use BB based cell then the DBEs should also return BB based cells. Currently they are byte[] array backed.

      Attachments

        1. HBASE-12374_v1.patch
          72 kB
          Anoop Sam John
        2. HBASE-12374_v2.patch
          72 kB
          Anoop Sam John
        3. HBASE-12374_v3.patch
          73 kB
          Anoop Sam John

        Activity

          anoop.hbase Anoop Sam John added a comment -

          The entire read path APIs to have this new Interface as param/return type rather than Cell. Till the RPC response.

          anoop.hbase Anoop Sam John added a comment - The entire read path APIs to have this new Interface as param/return type rather than Cell. Till the RPC response.
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12745953/HBASE-12374_v1.patch
          against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a.
          ATTACHMENT ID: 12745953

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

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

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0)

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +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 post-site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.io.hfile.TestSeekTo

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/checkstyle-aggregate.html

          Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/patchJavadocWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12745953/HBASE-12374_v1.patch against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a. ATTACHMENT ID: 12745953 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 18 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +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 post-site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.io.hfile.TestSeekTo Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14824//console This message is automatically generated.

          Will check the patch tomorrow. See if you can add TestSeekTo also to go thro this new offheap based testing.

          ram_krish ramkrishna.s.vasudevan added a comment - Will check the patch tomorrow. See if you can add TestSeekTo also to go thro this new offheap based testing.
          anoop.hbase Anoop Sam John added a comment -

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.io.hfile.TestSeekTo

          Related test failure. I fixed the patch so that test will pass.

          See if you can add TestSeekTo also to go thro this new offheap based testing.

          No we can not. This test passes the HFile block by reading the FS HFiles. The HFileReaderImpl is reading the file and to an on heap BB. We are not changing this area at all.. So no way to add off heap support for this test.

          anoop.hbase Anoop Sam John added a comment - -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.io.hfile.TestSeekTo Related test failure. I fixed the patch so that test will pass. See if you can add TestSeekTo also to go thro this new offheap based testing. No we can not. This test passes the HFile block by reading the FS HFiles. The HFileReaderImpl is reading the file and to an on heap BB. We are not changing this area at all.. So no way to add off heap support for this test.
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12745968/HBASE-12374_v2.patch
          against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a.
          ATTACHMENT ID: 12745968

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

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

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0)

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          -1 checkstyle. The applied patch generated 1872 checkstyle errors (more than the master's current 1871 errors).

          +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 post-site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.io.encoding.TestLoadAndSwitchEncodeOnDisk

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.camel.component.jpa.JpaWithNamedQueryTest.testProducerInsertsIntoDatabaseThenConsumerFiresMessageExchange(JpaWithNamedQueryTest.java:112)
          at org.apache.camel.component.jpa.JpaWithNamedQueryTest.testProducerInsertsIntoDatabaseThenConsumerFiresMessageExchange(JpaWithNamedQueryTest.java:112)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/checkstyle-aggregate.html

          Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/patchJavadocWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12745968/HBASE-12374_v2.patch against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a. ATTACHMENT ID: 12745968 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 18 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. -1 checkstyle . The applied patch generated 1872 checkstyle errors (more than the master's current 1871 errors). +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 post-site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.io.encoding.TestLoadAndSwitchEncodeOnDisk -1 core zombie tests . There are 1 zombie test(s): at org.apache.camel.component.jpa.JpaWithNamedQueryTest.testProducerInsertsIntoDatabaseThenConsumerFiresMessageExchange(JpaWithNamedQueryTest.java:112) at org.apache.camel.component.jpa.JpaWithNamedQueryTest.testProducerInsertsIntoDatabaseThenConsumerFiresMessageExchange(JpaWithNamedQueryTest.java:112) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14830//console This message is automatically generated.
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12746056/HBASE-12374_v3.patch
          against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a.
          ATTACHMENT ID: 12746056

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

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

          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0)

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

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

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

          +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

          +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 post-site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//testReport/
          Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//artifact/patchprocess/newFindbugsWarnings.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12746056/HBASE-12374_v3.patch against master branch at commit 338e39970ba8e4835733669b9252d073b2157b8a. ATTACHMENT ID: 12746056 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 18 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.7.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +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 post-site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/14834//console This message is automatically generated.

          I checked the patch end to end. LGTM.
          Great work.
          Can we rename the getKeyValue to getCell as part of this patch only. I think I did not do in the DBE case.

          ram_krish ramkrishna.s.vasudevan added a comment - I checked the patch end to end. LGTM. Great work. Can we rename the getKeyValue to getCell as part of this patch only. I think I did not do in the DBE case.

          One query,
          Will it be better to copy the tags part incase of tagCompressionContext == null && includeTags case - using a seperate asSubByteBuffer call rather than copying them in one call?

          ram_krish ramkrishna.s.vasudevan added a comment - One query, Will it be better to copy the tags part incase of tagCompressionContext == null && includeTags case - using a seperate asSubByteBuffer call rather than copying them in one call?
          anoop.hbase Anoop Sam John added a comment -

          Ya I can do the rename.

          Will it be better to copy the tags part incase of tagCompressionContext == null && includeTags case - using a seperate asSubByteBuffer call rather than copying them in one call?

          My thinking here was, mostly there wont be copy need for this asSubBuffer call. Even if copy is needed, we will do it is one call and make one object including both tags and value part (tags here is not compressed at all). The only case where this is an overhead (one call rather than 2 calls for value and tags) the value part ends exactly at one item BB and tags part starts in next BB. Then one call for value and tags will need a copy where as the other model dont need at all. Very rare chance.
          2 calls to asSubBuffer having overhead of calculations in MBB like finding the position and item BB. IMO current way is ok.

          anoop.hbase Anoop Sam John added a comment - Ya I can do the rename. Will it be better to copy the tags part incase of tagCompressionContext == null && includeTags case - using a seperate asSubByteBuffer call rather than copying them in one call? My thinking here was, mostly there wont be copy need for this asSubBuffer call. Even if copy is needed, we will do it is one call and make one object including both tags and value part (tags here is not compressed at all). The only case where this is an overhead (one call rather than 2 calls for value and tags) the value part ends exactly at one item BB and tags part starts in next BB. Then one call for value and tags will need a copy where as the other model dont need at all. Very rare chance. 2 calls to asSubBuffer having overhead of calculations in MBB like finding the position and item BB. IMO current way is ok.

          Okie. I think it makes sense.

          ram_krish ramkrishna.s.vasudevan added a comment - Okie. I think it makes sense.
          anoop.hbase Anoop Sam John added a comment -

          Thanks Ram. Will commit in some time.

          anoop.hbase Anoop Sam John added a comment - Thanks Ram. Will commit in some time.
          anoop.hbase Anoop Sam John added a comment -

          TEST-org.apache.hadoop.hbase.replication.TestReplicationKillSlaveRS.xml.<init>

          Test case failure is not related.

          anoop.hbase Anoop Sam John added a comment - TEST-org.apache.hadoop.hbase.replication.TestReplicationKillSlaveRS.xml.<init> Test case failure is not related.
          anoop.hbase Anoop Sam John added a comment -

          Pushed to master. Thanks Ram.

          anoop.hbase Anoop Sam John added a comment - Pushed to master. Thanks Ram.
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-TRUNK #6666 (See https://builds.apache.org/job/HBase-TRUNK/6666/)
          HBASE-12374 Change DBEs to work with new BB based cell. (anoopsamjohn: rev 0f614a1c44e1887ca7177b66bb6208b6e69db7e1)

          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestSeekToBlockWithEncoders.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBufferOutputStream.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DiffKeyDeltaEncoder.java
          • hbase-prefix-tree/src/main/java/org/apache/hadoop/hbase/codec/prefixtree/PrefixTreeSeeker.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestBufferedDataBlockEncoder.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/PrefixKeyDeltaEncoder.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/OffheapKeyOnlyKeyValue.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/StreamUtils.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestDataBlockEncoders.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestPrefixTreeEncoding.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/FastDiffDeltaEncoder.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
          • hbase-common/src/test/java/org/apache/hadoop/hbase/io/TestTagCompressionContext.java
          hudson Hudson added a comment - FAILURE: Integrated in HBase-TRUNK #6666 (See https://builds.apache.org/job/HBase-TRUNK/6666/ ) HBASE-12374 Change DBEs to work with new BB based cell. (anoopsamjohn: rev 0f614a1c44e1887ca7177b66bb6208b6e69db7e1) hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestSeekToBlockWithEncoders.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBufferOutputStream.java hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DiffKeyDeltaEncoder.java hbase-prefix-tree/src/main/java/org/apache/hadoop/hbase/codec/prefixtree/PrefixTreeSeeker.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestBufferedDataBlockEncoder.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/PrefixKeyDeltaEncoder.java hbase-common/src/main/java/org/apache/hadoop/hbase/OffheapKeyOnlyKeyValue.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/StreamUtils.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestDataBlockEncoders.java hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestPrefixTreeEncoding.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/FastDiffDeltaEncoder.java hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java hbase-common/src/test/java/org/apache/hadoop/hbase/io/TestTagCompressionContext.java

          People

            anoop.hbase Anoop Sam John
            ram_krish ramkrishna.s.vasudevan
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: