HBase
  1. HBase
  2. HBASE-7319

Extend Cell usage through read path

    Details

    • Hadoop Flags:
      Reviewed
    • Tags:
      Phoenix

      Description

      Umbrella issue for eliminating Cell copying.

      The Cell interface allows us to work with a reference to underlying bytes in the block cache without copying each Cell into consecutive bytes in an array (KeyValue).

      1. HBASE-7319.patch
        141 kB
        ramkrishna.s.vasudevan
      2. HBASE-7319_1.patch
        182 kB
        ramkrishna.s.vasudevan
      3. HBASE-7319_1.patch
        182 kB
        stack
      4. HBASE-7319_2.patch
        182 kB
        ramkrishna.s.vasudevan

        Issue Links

          Activity

          Hide
          Lars Hofhansl added a comment -

          We filed these at almost the exact same time

          Show
          Lars Hofhansl added a comment - We filed these at almost the exact same time
          Hide
          Matt Corgan added a comment -

          oops, sorry! i may have some sub-tasks that don't fit under your title though. fix as you see fit Lars.

          Show
          Matt Corgan added a comment - oops, sorry! i may have some sub-tasks that don't fit under your title though. fix as you see fit Lars.
          Hide
          Lars Hofhansl added a comment -

          I think the issue I filed would be a subtask under this one.

          Show
          Lars Hofhansl added a comment - I think the issue I filed would be a subtask under this one.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Once HBASE-10531 is in, will start with this. Replacing BRs in place of ByteBuffers preferably needs this change first.

          Show
          ramkrishna.s.vasudevan added a comment - Once HBASE-10531 is in, will start with this. Replacing BRs in place of ByteBuffers preferably needs this change first.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Changes KeyValue to Cell in the read path. ScanQueryMatch still uses KV. With this patch it would help BufferedDataBlockEncoders to use Cell and directly use it in comparison and during Kv.next() and kv.peek().

          Show
          ramkrishna.s.vasudevan added a comment - Changes KeyValue to Cell in the read path. ScanQueryMatch still uses KV. With this patch it would help BufferedDataBlockEncoders to use Cell and directly use it in comparison and during Kv.next() and kv.peek().
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 javadoc. The javadoc tool appears to have generated 7 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 introduces the following lines longer than 100:
          + return Bytes.equals(left.getQualifierArray(), left.getQualifierOffset(), left.getQualifierLength(),
          + int len = KeyValue.writeByteArray(buffer, boffset, row, roffset, rlength, family, foffset, flength,
          + (nextJoinedKv != null && CellUtil.matchingRow(nextJoinedKv, currentRow, offset, length))
          + || (this.joinedHeap.requestSeek(KeyValueUtil.createFirstOnRow(currentRow, offset, length),

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor
          org.apache.hadoop.hbase.regionserver.TestReversibleScanners
          org.apache.hadoop.hbase.client.TestFromClientSide

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//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/12639008/HBASE-7319.patch against trunk revision . ATTACHMENT ID: 12639008 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 45 new or modified tests. -1 javadoc . The javadoc tool appears to have generated 7 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 introduces the following lines longer than 100: + return Bytes.equals(left.getQualifierArray(), left.getQualifierOffset(), left.getQualifierLength(), + int len = KeyValue.writeByteArray(buffer, boffset, row, roffset, rlength, family, foffset, flength, + (nextJoinedKv != null && CellUtil.matchingRow(nextJoinedKv, currentRow, offset, length)) + || (this.joinedHeap.requestSeek(KeyValueUtil.createFirstOnRow(currentRow, offset, length), +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.client.TestFromClientSideWithCoprocessor org.apache.hadoop.hbase.regionserver.TestReversibleScanners org.apache.hadoop.hbase.client.TestFromClientSide Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9215//console This message is automatically generated.
          Hide
          stack added a comment -

          Tell us more ramkrishna.s.vasudevan about this 'ScanQueryMatch still uses KV. With this patch it would help BufferedDataBlockEncoders to use Cell and directly use it in comparison and during Kv.next() and kv.peek().' Will this be fixed in a follow up? Or these are not changeable?

          Show
          stack added a comment - Tell us more ramkrishna.s.vasudevan about this 'ScanQueryMatch still uses KV. With this patch it would help BufferedDataBlockEncoders to use Cell and directly use it in comparison and during Kv.next() and kv.peek().' Will this be fixed in a follow up? Or these are not changeable?
          Hide
          ramkrishna.s.vasudevan added a comment -

          ScanQueryMatcher is changable. For now it will still work with KeyValues. Will take it up in a follow up issue. It is a core area and so wanted to do it separately after making sure that these changes does not affect the other read areas.
          For the DBE part see HBASE-10801 and the patch attached there. This change combined with HBASE-10801 would speed up reads using DBE because we could avoid the deep copy while doing getKeyValue.
          See
          https://issues.apache.org/jira/browse/HBASE-10801?focusedCommentId=13941459&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13941459.

          Show
          ramkrishna.s.vasudevan added a comment - ScanQueryMatcher is changable. For now it will still work with KeyValues. Will take it up in a follow up issue. It is a core area and so wanted to do it separately after making sure that these changes does not affect the other read areas. For the DBE part see HBASE-10801 and the patch attached there. This change combined with HBASE-10801 would speed up reads using DBE because we could avoid the deep copy while doing getKeyValue. See https://issues.apache.org/jira/browse/HBASE-10801?focusedCommentId=13941459&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13941459 .
          Hide
          stack added a comment -

          Patch is looking good.

          These seem a little strange:

          public static KeyValue createFirstOnRow(final byte [] row)

          { - return createFirstOnRow(row, HConstants.LATEST_TIMESTAMP); + return KeyValueUtil.createFirstOnRow(row, HConstants.LATEST_TIMESTAMP); }

          Why not change the caller so it uses the KVU directly rather than have KV reference KVU?

          Same for createFirstOnRow, etc.

          I skimmed the patch. Radical. It looks great Ram.

          Show
          stack added a comment - Patch is looking good. These seem a little strange: public static KeyValue createFirstOnRow(final byte [] row) { - return createFirstOnRow(row, HConstants.LATEST_TIMESTAMP); + return KeyValueUtil.createFirstOnRow(row, HConstants.LATEST_TIMESTAMP); } Why not change the caller so it uses the KVU directly rather than have KV reference KVU? Same for createFirstOnRow, etc. I skimmed the patch. Radical. It looks great Ram.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Thanks for the review Stack. There are some test failures.
          Will udpate your comments too.
          Is it ok to rename getKeyValue() to getCell() in all the KeyValueScanner, HFileScanner etc? I have not done in the patch, but will it make sense?

          Show
          ramkrishna.s.vasudevan added a comment - Thanks for the review Stack. There are some test failures. Will udpate your comments too. Is it ok to rename getKeyValue() to getCell() in all the KeyValueScanner, HFileScanner etc? I have not done in the patch, but will it make sense?
          Hide
          ramkrishna.s.vasudevan added a comment -

          Failed testcases passes in this patch. Address the comments and also corrects the long lines.

          Show
          ramkrishna.s.vasudevan added a comment - Failed testcases passes in this patch. Address the comments and also corrects the long lines.
          Hide
          Hadoop QA added a comment -

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

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

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

          +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.client.TestMultiParallel

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//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/12639144/HBASE-7319_1.patch against trunk revision . ATTACHMENT ID: 12639144 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 63 new or modified tests. +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.client.TestMultiParallel Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9219//console This message is automatically generated.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Test failure seems unrelated. Passes locally.

          Show
          ramkrishna.s.vasudevan added a comment - Test failure seems unrelated. Passes locally.
          Hide
          Andrew Purtell added a comment -

          I spent about an hour with this patch today. Refactoring looks good. I like where we are now using static methods for comparisons. As transitional changes these look ok to me. I see some follow up issues have been filed already.

          Show
          Andrew Purtell added a comment - I spent about an hour with this patch today. Refactoring looks good. I like where we are now using static methods for comparisons. As transitional changes these look ok to me. I see some follow up issues have been filed already.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Will commit this patch unless objections. If there are more places to change will raise subsequent JIRAs to do that so that finally we can ensure that we deal with Cells rather than KVs.

          Show
          ramkrishna.s.vasudevan added a comment - Will commit this patch unless objections. If there are more places to change will raise subsequent JIRAs to do that so that finally we can ensure that we deal with Cells rather than KVs.
          Hide
          stack added a comment -

          ramkrishna.s.vasudevan Let me retry the hadoopqa to see if the failure related for sure or not. Good on you.

          Show
          stack added a comment - ramkrishna.s.vasudevan Let me retry the hadoopqa to see if the failure related for sure or not. Good on you.
          Hide
          Hadoop QA added a comment -

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

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

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

          +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 passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//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/12639340/HBASE-7319_1.patch against trunk revision . ATTACHMENT ID: 12639340 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 63 new or modified tests. +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 passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9227//console This message is automatically generated.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Will commit this patch?
          Anoop Sam John
          you have any comments on this?

          Show
          ramkrishna.s.vasudevan added a comment - Will commit this patch? Anoop Sam John you have any comments on this?
          Hide
          Anoop Sam John added a comment -

          Oh.. It is a big patch..Just started going through it Ram. One quick comment

                 KeyValue kv = KeyValueUtil.ensureKeyValue(kvs[i]);
          -      if (kv.matchingColumn(family,qualifier)) {
          +      if (CellUtil.matchingColumn(kv, family,qualifier)) {
          

          Now when u use CellUtil why still need KeyValueUtil.ensureKeyValue(kvs[i])? This is there in some places.

          Show
          Anoop Sam John added a comment - Oh.. It is a big patch..Just started going through it Ram. One quick comment KeyValue kv = KeyValueUtil.ensureKeyValue(kvs[i]); - if (kv.matchingColumn(family,qualifier)) { + if (CellUtil.matchingColumn(kv, family,qualifier)) { Now when u use CellUtil why still need KeyValueUtil.ensureKeyValue(kvs [i] )? This is there in some places.
          Hide
          ramkrishna.s.vasudevan added a comment -

          It is not needed. In my other patches I will remove most of the places where we try to use KeyValueUtil.ensureKeyValue(). If something is there in this current patch I can remove that too.

          Show
          ramkrishna.s.vasudevan added a comment - It is not needed. In my other patches I will remove most of the places where we try to use KeyValueUtil.ensureKeyValue(). If something is there in this current patch I can remove that too.
          Hide
          Anoop Sam John added a comment -

          Yes some 4, 5 places I have seen Ram. You can correct on commit.

          -    public KeyValue getKeyValue() {
          +    public Cell getKeyValue() {
          

          We need rename it as getCell() now?

          Other than these looks very good Ram.

          Show
          Anoop Sam John added a comment - Yes some 4, 5 places I have seen Ram. You can correct on commit. - public KeyValue getKeyValue() { + public Cell getKeyValue() { We need rename it as getCell() now? Other than these looks very good Ram.
          Hide
          ramkrishna.s.vasudevan added a comment -

          This is what I committed to trunk.

          Show
          ramkrishna.s.vasudevan added a comment - This is what I committed to trunk.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Thanks for the review Stack, Andrew and Anoop.

          Show
          ramkrishna.s.vasudevan added a comment - Thanks for the review Stack, Andrew and Anoop.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #5073 (See https://builds.apache.org/job/HBase-TRUNK/5073/)
          HBASE-7319-Extend Cell usage through read path (Ram) (ramkrishna: rev 1585945)

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPaginationFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPrefixFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnRangeFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FirstKeyValueMatchingQualifiersFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FuzzyRowFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/MultipleColumnPrefixFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueExcludeFilter.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java
          • /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataDeeper.java
          • /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataTrivial.java
          • /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataTrivialWithTags.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/Reference.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/WALPlayer.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/GetClosestRowBeforeTracker.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonReversedNonLazyKeyValueScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedKeyValueHeap.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedRegionScannerImpl.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/ParallelSeekHandler.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/Replication.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CollectionBackedScanner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterBase.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestDataBlockEncoders.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestPrefixTreeEncoding.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestSeekToBlockWithEncoders.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileEncryption.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/DataBlockEncodingTool.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/EncodedSeekPerformanceTest.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestReversibleScanners.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileScannerWithTagCompression.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #5073 (See https://builds.apache.org/job/HBase-TRUNK/5073/ ) HBASE-7319 -Extend Cell usage through read path (Ram) (ramkrishna: rev 1585945) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPaginationFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPrefixFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnRangeFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FirstKeyValueMatchingQualifiersFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FuzzyRowFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/MultipleColumnPrefixFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueExcludeFilter.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataDeeper.java /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataTrivial.java /hbase/trunk/hbase-prefix-tree/src/test/java/org/apache/hadoop/hbase/codec/prefixtree/row/data/TestRowDataTrivialWithTags.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/Reference.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/WALPlayer.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/GetClosestRowBeforeTracker.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonReversedNonLazyKeyValueScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedKeyValueHeap.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedRegionScannerImpl.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/ParallelSeekHandler.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogSplitter.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEditsReplaySink.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/Replication.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CollectionBackedScanner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CompoundBloomFilterBase.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/TestHalfStoreFileReader.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestDataBlockEncoders.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestPrefixTreeEncoding.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestSeekToBlockWithEncoders.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileEncryption.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestSeekTo.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestHFileOutputFormat.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/DataBlockEncodingTool.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/EncodedSeekPerformanceTest.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueScanFixture.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestReversibleScanners.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileScannerWithTagCompression.java
          Hide
          stack added a comment -

          ramkrishna.s.vasudevan You got Anoop Sam John's comment above? That seemed like a good one to do.

          Show
          stack added a comment - ramkrishna.s.vasudevan You got Anoop Sam John 's comment above? That seemed like a good one to do.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Stack
          You mean the comment to change to getCell?
          See my comment here https://issues.apache.org/jira/browse/HBASE-7319?focusedCommentId=13962605&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13962605. As I did not get a reply though can do it in a follow on JIRA (if needed).
          If it is the other comment I have done it already in the patch applied.

          Show
          ramkrishna.s.vasudevan added a comment - Stack You mean the comment to change to getCell? See my comment here https://issues.apache.org/jira/browse/HBASE-7319?focusedCommentId=13962605&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13962605 . As I did not get a reply though can do it in a follow on JIRA (if needed). If it is the other comment I have done it already in the patch applied.
          Hide
          stack added a comment -

          ramkrishna.s.vasudevan Follow-on is fine. Was talking about this:

          • public KeyValue getKeyValue() {
            + public Cell getKeyValue() {

          and

          We need rename it as getCell() now?

          Show
          stack added a comment - ramkrishna.s.vasudevan Follow-on is fine. Was talking about this: public KeyValue getKeyValue() { + public Cell getKeyValue() { and We need rename it as getCell() now?
          Hide
          ramkrishna.s.vasudevan added a comment -

          I would rename to getCell() in another JIRA after the pending things are done.

          Show
          ramkrishna.s.vasudevan added a comment - I would rename to getCell() in another JIRA after the pending things are done.
          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.

            People

            • Assignee:
              ramkrishna.s.vasudevan
              Reporter:
              Matt Corgan
            • Votes:
              0 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development