Details
-
Sub-task
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
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
Attachments
- HBASE-12374_v1.patch
- 72 kB
- Anoop Sam John
- HBASE-12374_v2.patch
- 72 kB
- Anoop Sam John
- HBASE-12374_v3.patch
- 73 kB
- Anoop Sam John
Activity
-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.
-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.
-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.
-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.
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?
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.
TEST-org.apache.hadoop.hbase.replication.TestReplicationKillSlaveRS.xml.<init>
Test case failure is not related.
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
The entire read path APIs to have this new Interface as param/return type rather than Cell. Till the RPC response.