HBase
  1. HBase
  2. HBASE-8012

Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.94.5
    • Fix Version/s: 0.98.0, 0.95.0
    • Component/s: regionserver
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The storeFileScanner's seekAtOrAfter method will position at the beginning of the file when the passed KV is smaller than first KV in file. While for reseekAtOrAfter, I think it should also do the same thing when it is the first time it been seeked. originally, this is workaround by adding a isReseekable property in StoreFileScanner, and is checked upon each enforceSeek(), if it is not seeked before, it will go with seek approaching instead of reseek approaching. While why not make reseekAtOrAfter working correctly for the first time it been reseek (also never been seek before), since the file is never seeked before, so position it at the beginning of the file don't break the idea of "reseek", say never rewind.

      It will save the effort for HBASE-8001, with this fixed, it won't need to check isReseekable any more.

      1. HBASE-8012_v2.patch
        4 kB
        Raymond Liu
      2. HBASE-8012.patch
        2 kB
        stack
      3. HBASE-8012.patch
        2 kB
        Raymond Liu

        Issue Links

          Activity

          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94 #1032 (See https://builds.apache.org/job/HBase-0.94/1032/)
          HBASE-8767 Backport hbase-8001 and hbase-8012, avoid lazy seek (original patches by Raymond Liu) (Revision 1498953)

          Result = SUCCESS

          Show
          Hudson added a comment - Integrated in HBase-0.94 #1032 (See https://builds.apache.org/job/HBase-0.94/1032/ ) HBASE-8767 Backport hbase-8001 and hbase-8012, avoid lazy seek (original patches by Raymond Liu) (Revision 1498953) Result = SUCCESS
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94-security #187 (See https://builds.apache.org/job/HBase-0.94-security/187/)
          HBASE-8767 Backport hbase-8001 and hbase-8012, avoid lazy seek (original patches by Raymond Liu) (Revision 1498953)

          Result = SUCCESS

          Show
          Hudson added a comment - Integrated in HBase-0.94-security #187 (See https://builds.apache.org/job/HBase-0.94-security/187/ ) HBASE-8767 Backport hbase-8001 and hbase-8012, avoid lazy seek (original patches by Raymond Liu) (Revision 1498953) Result = SUCCESS
          Hide
          Hudson added a comment -

          Integrated in hbase-0.95-on-hadoop2 #25 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/25/)
          HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455990)

          Result = FAILURE
          ramkrishna :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in hbase-0.95-on-hadoop2 #25 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/25/ ) HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455990) Result = FAILURE ramkrishna : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #446 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/446/)
          HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455993)

          Result = FAILURE
          ramkrishna :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #446 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/446/ ) HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455993) Result = FAILURE ramkrishna : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #3954 (See https://builds.apache.org/job/HBase-TRUNK/3954/)
          HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455993)

          Result = FAILURE
          ramkrishna :
          Files :

          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #3954 (See https://builds.apache.org/job/HBase-TRUNK/3954/ ) HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455993) Result = FAILURE ramkrishna : Files : /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          Hudson added a comment -

          Integrated in hbase-0.95 #69 (See https://builds.apache.org/job/hbase-0.95/69/)
          HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455990)

          Result = FAILURE
          ramkrishna :
          Files :

          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Show
          Hudson added a comment - Integrated in hbase-0.95 #69 (See https://builds.apache.org/job/hbase-0.95/69/ ) HBASE-8012 - Reseek should position to the beginning of file for the first time it is invoked with a KV smaller than the first KV in file (Raymond Liu) (Revision 1455990) Result = FAILURE ramkrishna : Files : /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
          Hide
          ramkrishna.s.vasudevan added a comment -

          Committed to 0.95 and trunk.
          Thanks Lars and Stack for the review.
          Thanks Liu for working on this patch.

          Show
          ramkrishna.s.vasudevan added a comment - Committed to 0.95 and trunk. Thanks Lars and Stack for the review. Thanks Liu for working on this patch.
          Hide
          stack added a comment -

          ramkrishna.s.vasudevan I am +1 for 0.95 and for trunk boss.

          Show
          stack added a comment - ramkrishna.s.vasudevan I am +1 for 0.95 and for trunk boss.
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Lars/Stack
          You are ok with the patch? I am +1 on this.
          Can we commit this?

          Show
          ramkrishna.s.vasudevan added a comment - @Lars/Stack You are ok with the patch? I am +1 on this. Can we commit this?
          Hide
          Raymond Liu added a comment -

          stack now QA test passed. Test case added. Anything else I need to do?

          Show
          Raymond Liu added a comment - stack now QA test passed. Test case added. Anything else I need to do?
          Hide
          Raymond Liu added a comment -

          oops. It just make reseekto 's behavior more correct and simulate the behavior of seekto -> It just make reseek 's behavior more correct and simulate the behavior of seek

          Show
          Raymond Liu added a comment - oops. It just make reseekto 's behavior more correct and simulate the behavior of seekto -> It just make reseek 's behavior more correct and simulate the behavior of seek
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12572710/HBASE-8012_v2.patch
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//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/12572710/HBASE-8012_v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4724//console This message is automatically generated.
          Hide
          Raymond Liu added a comment -

          stack : I think this patch won't do any performance benefit at present. It just make reseekto 's behavior more correct and simulate the behavior of seekto. it doesn't need to have extra place to keep tracking whether can we do reseek or not. reseek it self should always working correctly. ( since seek can why reseek should not). And I don't see downsides possibility, since the check need to be done anyway, if not inside reseek than before call reseek.

          While, as I mentioned before, If we are allowed to change the seekto and reseekto 's behavior, we can push this check into even lower level. at that case, could have micro level performance benefit. Since in some code branch, it won't need to be checked at all.

          Show
          Raymond Liu added a comment - stack : I think this patch won't do any performance benefit at present. It just make reseekto 's behavior more correct and simulate the behavior of seekto. it doesn't need to have extra place to keep tracking whether can we do reseek or not. reseek it self should always working correctly. ( since seek can why reseek should not). And I don't see downsides possibility, since the check need to be done anyway, if not inside reseek than before call reseek. While, as I mentioned before, If we are allowed to change the seekto and reseekto 's behavior, we can push this check into even lower level. at that case, could have micro level performance benefit. Since in some code branch, it won't need to be checked at all.
          Hide
          stack added a comment -

          Raymond Liu Can you think of any possible downsides? What are the advantages of the patch? The removal of isReseekable, a boolean we don't need since you can infer the state? I do not know this code well so excuse my asking these questions. Any performance benefit that you see? Thank you.

          Show
          stack added a comment - Raymond Liu Can you think of any possible downsides? What are the advantages of the patch? The removal of isReseekable, a boolean we don't need since you can infer the state? I do not know this code well so excuse my asking these questions. Any performance benefit that you see? Thank 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/12572701/HBASE-8012.patch
          against trunk revision .

          +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 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//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/12572701/HBASE-8012.patch against trunk revision . +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 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4723//console This message is automatically generated.
          Hide
          Raymond Liu added a comment -

          Updated with a test case cover the scenario that reseek is call with a empty KV before any real seek is done. Should be able to position at the beginning of the file correctly instead of return NULL upon next()

          Show
          Raymond Liu added a comment - Updated with a test case cover the scenario that reseek is call with a empty KV before any real seek is done. Should be able to position at the beginning of the file correctly instead of return NULL upon next()
          Hide
          stack added a comment -

          Retry to see if failures are because of this patch

          Show
          stack added a comment - Retry to see if failures are because of this patch
          Hide
          ramkrishna.s.vasudevan added a comment -

          The reasoning behind the patch seems right.

          Show
          ramkrishna.s.vasudevan added a comment - The reasoning behind the patch seems right.
          Hide
          Ted Yu added a comment -

          Otherwise, if it is called without position to the first kv, Yes, the file will be skipped and scanner closed. But it is wrong behavior since the file actually not finished.

          Can you add a test for the above case ?

          Thanks

          Show
          Ted Yu added a comment - Otherwise, if it is called without position to the first kv, Yes, the file will be skipped and scanner closed. But it is wrong behavior since the file actually not finished. Can you add a test for the above case ? Thanks
          Hide
          Hadoop QA added a comment -

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

          +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 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

          +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 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.catalog.TestMetaMigrationConvertingToPB
          org.apache.hadoop.hbase.replication.TestReplicationQueueFailoverCompressed
          org.apache.hadoop.hbase.replication.TestReplicationQueueFailover
          org.apache.hadoop.hbase.client.TestFromClientSide
          org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//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/12572261/HBASE-8012.patch against trunk revision . +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 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +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 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.catalog.TestMetaMigrationConvertingToPB org.apache.hadoop.hbase.replication.TestReplicationQueueFailoverCompressed org.apache.hadoop.hbase.replication.TestReplicationQueueFailover org.apache.hadoop.hbase.client.TestFromClientSide org.apache.hadoop.hbase.regionserver.TestSplitTransactionOnCluster Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4693//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Thanks Raymond. Makes sense.
          Mikhail Bautin, wanna have a look? You're the lazy-seek expert

          Show
          Lars Hofhansl added a comment - Thanks Raymond. Makes sense. Mikhail Bautin , wanna have a look? You're the lazy-seek expert
          Hide
          Raymond Liu added a comment -

          Lars Hofhansl: I think it won't. a lazy seek could have the chance to skip the entire file. If it already come down to reseek, the reseek result had to be sucessful and correct. This could be achieved either by: reseek will be correct by itself, or like the original solution it should be changed into a seek op instead of reseek, since reseek won't work correctly under this condition. Otherwise, if it is called without position to the first kv, Yes, the file will be skipped and scanner closed. But it is wrong behavior since the file actually not finished.

          Show
          Raymond Liu added a comment - Lars Hofhansl : I think it won't. a lazy seek could have the chance to skip the entire file. If it already come down to reseek, the reseek result had to be sucessful and correct. This could be achieved either by: reseek will be correct by itself, or like the original solution it should be changed into a seek op instead of reseek, since reseek won't work correctly under this condition. Otherwise, if it is called without position to the first kv, Yes, the file will be skipped and scanner closed. But it is wrong behavior since the file actually not finished.
          Hide
          Lars Hofhansl added a comment -

          Thanks for digging in Raymond.
          Wouldn't this patch imply that we are seeking into each StoreFile at least once? It might be that a (re)seek could skip the entire file.

          Show
          Lars Hofhansl added a comment - Thanks for digging in Raymond. Wouldn't this patch imply that we are seeking into each StoreFile at least once? It might be that a (re)seek could skip the entire file.
          Hide
          Raymond Liu added a comment -

          stack would you help to take a review on this.

          Show
          Raymond Liu added a comment - stack would you help to take a review on this.
          Hide
          Raymond Liu added a comment -

          It could be done at the keyValueScanner level with less chance of requiring to check the isSeeked(), while if we do not want to change the keyValueScanner's meaning upon return value from seekto and reseekto, then this check had to be done at storefilescanner level which do need to be check every time though it had been checked by keyValueScanner due to the loss of information ( <0 could be returned by abstractScannerV2 from multiple place with actually different meaning)

          Show
          Raymond Liu added a comment - It could be done at the keyValueScanner level with less chance of requiring to check the isSeeked(), while if we do not want to change the keyValueScanner's meaning upon return value from seekto and reseekto, then this check had to be done at storefilescanner level which do need to be check every time though it had been checked by keyValueScanner due to the loss of information ( <0 could be returned by abstractScannerV2 from multiple place with actually different meaning)

            People

            • Assignee:
              Raymond Liu
              Reporter:
              Raymond Liu
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development