Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-13109

Make better SEEK vs SKIP decisions during scanning

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0, 1.0.1, 1.1.0, 0.98.12
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      I'm re-purposing this issue to add a heuristic as to when to SEEK and when to SKIP Cells. This has come up in various issues, and I think I have a way to finally fix this now. HBASE-9778, HBASE-12311, and friends are related.

      — Old description —

      This is a continuation of HBASE-9778.
      We've seen a scenario of a very slow scan over a region using a timerange that happens to fall after the ts of any Cell in the region.
      Turns out we spend a lot of time seeking.

      Tested with a 5 column table, and the scan is 5x faster when the timerange falls before all Cells' ts.
      We can use the lookahead hint introduced in HBASE-9778 to do opportunistic SKIPing before we actually seek.

      1. 13109-0.98-v5.txt
        27 kB
        Lars Hofhansl
      2. 13109-trunk-v5.txt
        35 kB
        Lars Hofhansl
      3. 13109-0.98-v4.txt
        27 kB
        Lars Hofhansl
      4. nextIndexKVChange_new.patch
        6 kB
        ramkrishna.s.vasudevan
      5. 13109-trunk-v4.txt
        25 kB
        Lars Hofhansl
      6. 13109-trunk-v3.txt
        24 kB
        Lars Hofhansl
      7. 13109-trunk-v2.txt
        24 kB
        Lars Hofhansl
      8. 13109-trunk.txt
        27 kB
        Lars Hofhansl

        Issue Links

          Activity

          Hide
          enis Enis Soztutar added a comment -

          Closing this issue after 1.0.1 release.

          Show
          enis Enis Soztutar added a comment - Closing this issue after 1.0.1 release.
          Hide
          hudson Hudson added a comment -

          ABORTED: Integrated in Phoenix-master #638 (See https://builds.apache.org/job/Phoenix-master/638/)
          PHOENIX-1642 Make Phoenix Master Branch pointing to HBase1.0.0 - ADDENDUM for HBASE-13109 (enis: rev ad2ad0cefd5d19a9bc84345444455a9ecbb55c78)

          • phoenix-core/src/main/java/org/apache/phoenix/hbase/index/scanner/FilteredKeyValueScanner.java
          • phoenix-core/src/main/java/org/apache/hadoop/hbase/regionserver/IndexHalfStoreFileReader.java
          Show
          hudson Hudson added a comment - ABORTED: Integrated in Phoenix-master #638 (See https://builds.apache.org/job/Phoenix-master/638/ ) PHOENIX-1642 Make Phoenix Master Branch pointing to HBase1.0.0 - ADDENDUM for HBASE-13109 (enis: rev ad2ad0cefd5d19a9bc84345444455a9ecbb55c78) phoenix-core/src/main/java/org/apache/phoenix/hbase/index/scanner/FilteredKeyValueScanner.java phoenix-core/src/main/java/org/apache/hadoop/hbase/regionserver/IndexHalfStoreFileReader.java
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Vikas Vishwakarma confirmed that the scan mentioned in the description is about 3x faster. That's a 3x end-to-end improvement in an M/R job!

          Show
          lhofhansl Lars Hofhansl added a comment - Vikas Vishwakarma confirmed that the scan mentioned in the description is about 3x faster. That's a 3x end-to-end improvement in an M/R job!
          Hide
          lhofhansl Lars Hofhansl added a comment -

          For the record... The full patch (with followup javadoc fixes) committed to 0.98.

          Show
          lhofhansl Lars Hofhansl added a comment - For the record... The full patch (with followup javadoc fixes) committed to 0.98.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          But the HBase code would call into the scanners injected by the Phoenix coprocessors... Anyway, since it works fine, there's something I am missing, which is just fine

          Show
          lhofhansl Lars Hofhansl added a comment - But the HBase code would call into the scanners injected by the Phoenix coprocessors... Anyway, since it works fine, there's something I am missing, which is just fine
          Hide
          apurtell Andrew Purtell added a comment -

          I admit I am surprised, since the HBase core code would call the new method on the HFileScanner and KeyValueScanner interfaces

          Phoenix code does not call the new method, of course, which is why the binary compatibility checker didn't flag this as a problem. It would have been a different story if there was a removal or rename of a specific method used by Phoenix.

          Show
          apurtell Andrew Purtell added a comment - I admit I am surprised, since the HBase core code would call the new method on the HFileScanner and KeyValueScanner interfaces Phoenix code does not call the new method, of course, which is why the binary compatibility checker didn't flag this as a problem. It would have been a different story if there was a removal or rename of a specific method used by Phoenix.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Pffeeewww... Good. (I admit I am surprised, since the HBase core code would call the new method on the HFileScanner and KeyValueScanner interfaces)

          The fact remains, though, that we have to get a new version of Phoenix 4.3 and 4.2 out before we ship 0.98.12, else there would be no released version of Phoenix to compile against the current version of 0.98. I discussed offline with Andrew Purtell, and we think that might the best option. I assume if we can't make that, we'll delay this until 0.98.13.

          (Sorry for the pain caused here. It's instructive, though, and worth the performance gains)

          Show
          lhofhansl Lars Hofhansl added a comment - Pffeeewww... Good. (I admit I am surprised, since the HBase core code would call the new method on the HFileScanner and KeyValueScanner interfaces) The fact remains, though, that we have to get a new version of Phoenix 4.3 and 4.2 out before we ship 0.98.12, else there would be no released version of Phoenix to compile against the current version of 0.98. I discussed offline with Andrew Purtell , and we think that might the best option. I assume if we can't make that, we'll delay this until 0.98.13. (Sorry for the pain caused here. It's instructive, though, and worth the performance gains)
          Hide
          mujtabachohan Mujtaba Chohan added a comment -

          James Taylor Checked. Mutable/local index using existing 4.3.0 release works fine with 0.98.12-SNAPSHOT.

          Show
          mujtabachohan Mujtaba Chohan added a comment - James Taylor Checked. Mutable/local index using existing 4.3.0 release works fine with 0.98.12-SNAPSHOT.
          Hide
          apurtell Andrew Purtell added a comment -

          Re-resolving. Follow up in PHOENIX-1731

          Show
          apurtell Andrew Purtell added a comment - Re-resolving. Follow up in PHOENIX-1731
          Hide
          jamestaylor James Taylor added a comment -

          Thanks, Andrew Purtell. Mujtaba Chohan - would you mind trying existing Phoenix binary release (4.3.0 is fine) against this snapshot. In particular, do mutable and local indexing still work correctly?

          Show
          jamestaylor James Taylor added a comment - Thanks, Andrew Purtell . Mujtaba Chohan - would you mind trying existing Phoenix binary release (4.3.0 is fine) against this snapshot. In particular, do mutable and local indexing still work correctly?
          Hide
          apurtell Andrew Purtell added a comment -

          I've confirmed 0.98.12 snapshots are available in Apache Maven now.

          Be sure the Apache snapshots repository is included in your POM:

              <repository>
                <id>apache.snapshots</id>
                <url>http://repository.apache.org/snapshots/</url>
                <snapshots>
                  <enabled>true</enabled>
                </snapshots>
              </repository>
          

          Use the versions "0.98.12-hadoop2-SNAPSHOT" for the Hadoop 2 build, "0.98.12-hadoop1-SNAPSHOT" for the Hadoop 1 build.

          Show
          apurtell Andrew Purtell added a comment - I've confirmed 0.98.12 snapshots are available in Apache Maven now. Be sure the Apache snapshots repository is included in your POM: <repository> <id>apache.snapshots</id> <url>http://repository.apache.org/snapshots/</url> <snapshots> <enabled>true</enabled> </snapshots> </repository> Use the versions "0.98.12-hadoop2-SNAPSHOT" for the Hadoop 2 build, "0.98.12-hadoop1-SNAPSHOT" for the Hadoop 1 build.
          Hide
          apurtell Andrew Purtell added a comment -

          I'll publish a snapshot today, so you can do either.

          Show
          apurtell Andrew Purtell added a comment - I'll publish a snapshot today, so you can do either.
          Hide
          stack stack added a comment -

          Would it be possible to get the 0.98.12 snapshot into maven so we can see if/how Phoenix will work with it?

          Can't you not build it local? This will install it in your local repo. You can then check phoenix against the locally installed version?

          Show
          stack stack added a comment - Would it be possible to get the 0.98.12 snapshot into maven so we can see if/how Phoenix will work with it? Can't you not build it local? This will install it in your local repo. You can then check phoenix against the locally installed version?
          Hide
          jamestaylor James Taylor added a comment -

          Would it be possible to get the 0.98.12 snapshot into maven so we can see if/how Phoenix will work with it? We plan on releasing a 4.3.1 soon - perhaps we can start a vote in a week. What's your time frame for 0.98.12?

          Show
          jamestaylor James Taylor added a comment - Would it be possible to get the 0.98.12 snapshot into maven so we can see if/how Phoenix will work with it? We plan on releasing a 4.3.1 soon - perhaps we can start a vote in a week. What's your time frame for 0.98.12?
          Hide
          lhofhansl Lars Hofhansl added a comment - - edited

          James Taylor, what do you think? If we delay to 0.98.13, I think we can have new versions of Phoenix by then.
          (If not, might as well leave it in 0.98.12)

          We should also check a version Phoenix built against an 0.98.11, and then run it against a version of HBase with this patch. I guess the only case in which we'd have an issue is with indexes.

          Show
          lhofhansl Lars Hofhansl added a comment - - edited James Taylor , what do you think? If we delay to 0.98.13, I think we can have new versions of Phoenix by then. (If not, might as well leave it in 0.98.12) We should also check a version Phoenix built against an 0.98.11, and then run it against a version of HBase with this patch. I guess the only case in which we'd have an issue is with indexes.
          Hide
          apurtell Andrew Purtell added a comment -

          And -1 for permanent revert

          Show
          apurtell Andrew Purtell added a comment - And -1 for permanent revert
          Hide
          apurtell Andrew Purtell added a comment -

          Thanks for checking. I'd be +0 on a delay as well

          Show
          apurtell Andrew Purtell added a comment - Thanks for checking. I'd be +0 on a delay as well
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Ultimately I only need this method in: KeyValueHeap, StoreScanner, StoreFileScanner, and AbstractHFileReader.Scanner. But doing only would litter class casts all over the code along with class checks in hot code paths. I do not think that makes sense.

          So, we can undo this from 0.98 altogether. (but -1 on that from me). Or we can delay this until 0.98.13. By then Phoenix needs to have new minor versions of 4.2 and 4.3. I'd be +0 on that. And just in case, -1 on delaying this further than 0.98.13...

          Show
          lhofhansl Lars Hofhansl added a comment - Ultimately I only need this method in: KeyValueHeap, StoreScanner, StoreFileScanner, and AbstractHFileReader.Scanner. But doing only would litter class casts all over the code along with class checks in hot code paths. I do not think that makes sense. So, we can undo this from 0.98 altogether. (but -1 on that from me). Or we can delay this until 0.98.13. By then Phoenix needs to have new minor versions of 4.2 and 4.3. I'd be +0 on that. And just in case, -1 on delaying this further than 0.98.13...
          Hide
          lhofhansl Lars Hofhansl added a comment -

          I do not think I can fix this without the extra methods. Lemme have a look.
          We can also undo this for 0.98.12 and put it into 0.98.13 instead, which time there should be new point versions of Phoenix.

          Show
          lhofhansl Lars Hofhansl added a comment - I do not think I can fix this without the extra methods. Lemme have a look. We can also undo this for 0.98.12 and put it into 0.98.13 instead, which time there should be new point versions of Phoenix.
          Hide
          apurtell Andrew Purtell added a comment -

          Let's see what Lars says first. I'd like to accommodate Phoenix if we can.

          Show
          apurtell Andrew Purtell added a comment - Let's see what Lars says first. I'd like to accommodate Phoenix if we can.
          Hide
          jamestaylor James Taylor added a comment -

          Not positive it's broken, so we should try it for sure. Is it available in maven?

          Yes, I meant the combined HBase+Phoenix community. Of course it's absolutely your call - would just be good if we knew in advance.

          Show
          jamestaylor James Taylor added a comment - Not positive it's broken, so we should try it for sure. Is it available in maven? Yes, I meant the combined HBase+Phoenix community. Of course it's absolutely your call - would just be good if we knew in advance.
          Hide
          apurtell Andrew Purtell added a comment -

          James Taylor JavaACC says the changes have no binary compat impact with already compiled code. I don't see how previous releases of Phoenix are broken. I think that states the problem too strongly. It is true that recompilation will be problematic without accommodation (see above) or local patching, but that's not the same thing as broken, right?

          Show
          apurtell Andrew Purtell added a comment - James Taylor JavaACC says the changes have no binary compat impact with already compiled code. I don't see how previous releases of Phoenix are broken. I think that states the problem too strongly. It is true that recompilation will be problematic without accommodation (see above) or local patching, but that's not the same thing as broken, right?
          Hide
          apurtell Andrew Purtell added a comment -

          Which user community? HBase is the operative one here. Of which phoenix users are a part, of course.

          Lars Hofhansl Can this change be implemented without adding the new member to this interface?

          If yes, we should fix Phoenix compilation issues as an accommodation.

          If no, well these are private annotated interfaces after all and this is a nice perf gain for HBase users, so as 0.98 RM I'll call it as good to stay.

          Show
          apurtell Andrew Purtell added a comment - Which user community? HBase is the operative one here. Of which phoenix users are a part, of course. Lars Hofhansl Can this change be implemented without adding the new member to this interface? If yes, we should fix Phoenix compilation issues as an accommodation. If no, well these are private annotated interfaces after all and this is a nice perf gain for HBase users, so as 0.98 RM I'll call it as good to stay.
          Hide
          jamestaylor James Taylor added a comment -

          I believe this essentially breaks all existing 4.x Phoenix releases (at a minimum, it would break mutable and local secondary indexes). The only way Phoenix users will be able to use 0.98.12+ is to wait for the next Phoenix release and upgrade to that one (at a minimum on the server side). Not sure if this is a problem for the user community.

          Show
          jamestaylor James Taylor added a comment - I believe this essentially breaks all existing 4.x Phoenix releases (at a minimum, it would break mutable and local secondary indexes). The only way Phoenix users will be able to use 0.98.12+ is to wait for the next Phoenix release and upgrade to that one (at a minimum on the server side). Not sure if this is a problem for the user community.
          Hide
          apurtell Andrew Purtell added a comment - - edited

          So here's where I think we are:

          • This commit is fine.
          • No new base classes and inheritance hierarchy changes
          • Handle updates to Phoenix on a Phoenix JIRA. I can push a 0.98.12 SNAPSHOT to Maven if that would help.

          Any issues with this James Taylor Lars Hofhansl ?

          Show
          apurtell Andrew Purtell added a comment - - edited So here's where I think we are: This commit is fine. No new base classes and inheritance hierarchy changes Handle updates to Phoenix on a Phoenix JIRA. I can push a 0.98.12 SNAPSHOT to Maven if that would help. Any issues with this James Taylor Lars Hofhansl ?
          Hide
          lhofhansl Lars Hofhansl added a comment -

          It'd be best to add the methods to Phoenix. If we do not add an override annotation that would work with old and new versions of HBase.

          Show
          lhofhansl Lars Hofhansl added a comment - It'd be best to add the methods to Phoenix. If we do not add an override annotation that would work with old and new versions of HBase.
          Hide
          apurtell Andrew Purtell added a comment -

          If we add base classes and change the inheritance hierarchy that may impact binary compat.

          Show
          apurtell Andrew Purtell added a comment - If we add base classes and change the inheritance hierarchy that may impact binary compat.
          Hide
          apurtell Andrew Purtell added a comment -

          Want to make sure I understand the b/w compat implications. Does this mean that our current 4.3 and below releases will no longer work with 0.98.12 and above? And that 4.4 and above will only work with 0.98.12 and above?

          <= 4.3 won't compile.

          I ran the binary compatibility checker and it says the addition of the abstract method getNextIndexedKey( ) to the interfaces has no effect.

          Show
          apurtell Andrew Purtell added a comment - Want to make sure I understand the b/w compat implications. Does this mean that our current 4.3 and below releases will no longer work with 0.98.12 and above? And that 4.4 and above will only work with 0.98.12 and above? <= 4.3 won't compile. I ran the binary compatibility checker and it says the addition of the abstract method getNextIndexedKey( ) to the interfaces has no effect.
          Hide
          jamestaylor James Taylor added a comment -

          Want to make sure I understand the b/w compat implications. Does this mean that our current 4.3 and below releases will no longer work with 0.98.12 and above? And that 4.4 and above will only work with 0.98.12 and above?

          Show
          jamestaylor James Taylor added a comment - Want to make sure I understand the b/w compat implications. Does this mean that our current 4.3 and below releases will no longer work with 0.98.12 and above? And that 4.4 and above will only work with 0.98.12 and above?
          Hide
          apurtell Andrew Purtell added a comment -

          Also, see above my suggestion to move to a 0.98 SNAPSHOT after making the change. Or, I can just ignore that Phoenix won't compile until updated with the 0.98.12 RC when working on the next release.

          Show
          apurtell Andrew Purtell added a comment - Also, see above my suggestion to move to a 0.98 SNAPSHOT after making the change. Or, I can just ignore that Phoenix won't compile until updated with the 0.98.12 RC when working on the next release.
          Hide
          apurtell Andrew Purtell added a comment - - edited

          Would it be possible to have base classes for these we can extend from to shield us from interface additions?

          Yes we can add these, but it's still one incompat change to move to using the base classes. Ok?

          Edit: And the base classes will still be Private because the interface is private.

          Show
          apurtell Andrew Purtell added a comment - - edited Would it be possible to have base classes for these we can extend from to shield us from interface additions? Yes we can add these, but it's still one incompat change to move to using the base classes. Ok? Edit: And the base classes will still be Private because the interface is private.
          Hide
          jamestaylor James Taylor added a comment -

          Would it be possible to have base classes for these we can extend from to shield us from interface additions?

          The FilteredKeyValueScanner class is deep in the bowels of mutable secondary indexing - Jesse Yates - any ideas for how to get this on to non private/evolving interfaces?

          The HFileScanner anonymous implementation is in the bowels of local indexes. Same question, Rajeshbabu Chintaguntla - any ideas for how to get this on to non private/evolving interfaces?

          Show
          jamestaylor James Taylor added a comment - Would it be possible to have base classes for these we can extend from to shield us from interface additions? The FilteredKeyValueScanner class is deep in the bowels of mutable secondary indexing - Jesse Yates - any ideas for how to get this on to non private/evolving interfaces? The HFileScanner anonymous implementation is in the bowels of local indexes. Same question, Rajeshbabu Chintaguntla - any ideas for how to get this on to non private/evolving interfaces?
          Hide
          apurtell Andrew Purtell added a comment -

          I would rather not impose a performance penalty on 0.98 for sake of compatibility with Phoenix where they've used a private interface. When I do 0.98 RC I check if Phoenix compiles using the heads of master and sometimes branch 4.0 if I have time. We could get that working today actually. I could publish a 0.98.12 SNAPSHOT to Maven now including this change, and update Phoenix POMs on branches master and 4.0 to use the snapshot, and add the necessary methods there. James Taylor?

          Show
          apurtell Andrew Purtell added a comment - I would rather not impose a performance penalty on 0.98 for sake of compatibility with Phoenix where they've used a private interface. When I do 0.98 RC I check if Phoenix compiles using the heads of master and sometimes branch 4.0 if I have time. We could get that working today actually. I could publish a 0.98.12 SNAPSHOT to Maven now including this change, and update Phoenix POMs on branches master and 4.0 to use the snapshot, and add the necessary methods there. James Taylor ?
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Hmm... Not sure what we can do other than (1) by using a private/evolving interface you're on your own or (2) roll this back from 0.98.
          I'd be fine with #2.

          I suppose we could add implementations of these methods In Phoenix now (it's perfectly OK to just return null, just means this optimization will not be used).

          Show
          lhofhansl Lars Hofhansl added a comment - Hmm... Not sure what we can do other than (1) by using a private/evolving interface you're on your own or (2) roll this back from 0.98. I'd be fine with #2. I suppose we could add implementations of these methods In Phoenix now (it's perfectly OK to just return null, just means this optimization will not be used).
          Hide
          apurtell Andrew Purtell added a comment -
          Show
          apurtell Andrew Purtell added a comment - Jesse Yates
          Hide
          apurtell Andrew Purtell added a comment -

          The commit of this to 0.98 branch breaks Phoenix compilation if using -Dhbase.version=0.98.12-SNAPSHOT (after installing latest 0.98 into the local Maven cache):

          [ERROR] /Users/apurtell/src/phoenix/phoenix-core/src/main/java/org/apache/hadoop/hbase/regionserver/IndexHalfStoreFileReader.java:[141,35] <anonymous org.apache.hadoop.hbase.regionserver.IndexHalfStoreFileReader$1> is not abstract and does not override abstract method getNextIndexedKey() in org.apache.hadoop.hbase.io.hfile.HFileScanner
          [ERROR] /Users/apurtell/src/phoenix/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/scanner/FilteredKeyValueScanner.java:[37,8] org.apache.phoenix.hbase.index.scanner.FilteredKeyValueScanner is not abstract and does not override abstract method getNextIndexedKey() in org.apache.hadoop.hbase.regionserver.KeyValueScanner
          

          KeyValueScanner and HFileScanner are both marked as InterfaceAudience.Private. What should we do here?

          James Taylor

          Show
          apurtell Andrew Purtell added a comment - The commit of this to 0.98 branch breaks Phoenix compilation if using -Dhbase.version=0.98.12-SNAPSHOT (after installing latest 0.98 into the local Maven cache): [ERROR] /Users/apurtell/src/phoenix/phoenix-core/src/main/java/org/apache/hadoop/hbase/regionserver/IndexHalfStoreFileReader.java:[141,35] <anonymous org.apache.hadoop.hbase.regionserver.IndexHalfStoreFileReader$1> is not abstract and does not override abstract method getNextIndexedKey() in org.apache.hadoop.hbase.io.hfile.HFileScanner [ERROR] /Users/apurtell/src/phoenix/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/scanner/FilteredKeyValueScanner.java:[37,8] org.apache.phoenix.hbase.index.scanner.FilteredKeyValueScanner is not abstract and does not override abstract method getNextIndexedKey() in org.apache.hadoop.hbase.regionserver.KeyValueScanner KeyValueScanner and HFileScanner are both marked as InterfaceAudience.Private. What should we do here? James Taylor
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #841 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/841/)
          HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 2eda262dfee9889a008cb53d5c8a2a73959934e4)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #841 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/841/ ) HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 2eda262dfee9889a008cb53d5c8a2a73959934e4) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-0.98 #884 (See https://builds.apache.org/job/HBase-0.98/884/)
          HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 2eda262dfee9889a008cb53d5c8a2a73959934e4)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-0.98 #884 (See https://builds.apache.org/job/HBase-0.98/884/ ) HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 2eda262dfee9889a008cb53d5c8a2a73959934e4) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.0 #789 (See https://builds.apache.org/job/HBase-1.0/789/)
          HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev d72bb2f6a60bdf2ac9daf639f18030eee2ea9773)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.0 #789 (See https://builds.apache.org/job/HBase-1.0/789/ ) HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev d72bb2f6a60bdf2ac9daf639f18030eee2ea9773) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #6207 (See https://builds.apache.org/job/HBase-TRUNK/6207/)
          HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 0bdab85b065bd0876152ac30c2ec6d08adae8006)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #6207 (See https://builds.apache.org/job/HBase-TRUNK/6207/ ) HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 0bdab85b065bd0876152ac30c2ec6d08adae8006) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #840 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/840/)
          HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev b3bd0016492eb99e3a83353f0879bfddebff4ec1)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-0.98-on-Hadoop-1.1 #840 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/840/ ) HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev b3bd0016492eb99e3a83353f0879bfddebff4ec1) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.1 #249 (See https://builds.apache.org/job/HBase-1.1/249/)
          HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 1cdcb6e9b8d386d43b482ff8a5aa6f1c0e3c6791)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.1 #249 (See https://builds.apache.org/job/HBase-1.1/249/ ) HBASE-13109 Fix Javadoc warning; and some misc checkstyle warnings (larsh: rev 1cdcb6e9b8d386d43b482ff8a5aa6f1c0e3c6791) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Updated all branches.

          Show
          lhofhansl Lars Hofhansl added a comment - Updated all branches.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-0.98 #883 (See https://builds.apache.org/job/HBase-0.98/883/)
          HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev b3bd0016492eb99e3a83353f0879bfddebff4ec1)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #883 (See https://builds.apache.org/job/HBase-0.98/883/ ) HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev b3bd0016492eb99e3a83353f0879bfddebff4ec1) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          Hide
          lhofhansl Lars Hofhansl added a comment - - edited

          Uh oh... Lemme fix those. Somehow I missed those in the test runs.
          There's also the check style warning... I'm getting sloppy.

          Show
          lhofhansl Lars Hofhansl added a comment - - edited Uh oh... Lemme fix those. Somehow I missed those in the test runs. There's also the check style warning... I'm getting sloppy.
          Hide
          jonathan.lawlor Jonathan Lawlor added a comment -

          Looks like this introduced some new javadoc warnings that are being called out in other precommit build checks:

          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java:82: warning - @param argument "lookAhead" is not a parameter name.
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:587: warning - @param argument "off" is not a parameter name.
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:587: warning - @param argument "len" is not a parameter name.
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:602: warning - @param argument "off" is not a parameter name.
          [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:602: warning - @param argument "len" is not a parameter name.

          Show
          jonathan.lawlor Jonathan Lawlor added a comment - Looks like this introduced some new javadoc warnings that are being called out in other precommit build checks: [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java:82: warning - @param argument "lookAhead" is not a parameter name. [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:587: warning - @param argument "off" is not a parameter name. [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:587: warning - @param argument "len" is not a parameter name. [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:602: warning - @param argument "off" is not a parameter name. [WARNING] /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java:602: warning - @param argument "len" is not a parameter name.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.1 #247 (See https://builds.apache.org/job/HBase-1.1/247/)
          HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev f5020e9c1a98727cb100f24294df50072d599bf8)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.1 #247 (See https://builds.apache.org/job/HBase-1.1/247/ ) HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev f5020e9c1a98727cb100f24294df50072d599bf8) hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.0 #788 (See https://builds.apache.org/job/HBase-1.0/788/)
          HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev a3e9325150de4ad89f3032535be8e20fb352f182)

          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.0 #788 (See https://builds.apache.org/job/HBase-1.0/788/ ) HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev a3e9325150de4ad89f3032535be8e20fb352f182) hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #6205 (See https://builds.apache.org/job/HBase-TRUNK/6205/)
          HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev 464e7ce685486e3ede13ec2351b45b0a0b65696c)

          • hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #6205 (See https://builds.apache.org/job/HBase-TRUNK/6205/ ) HBASE-13109 Make better SEEK vs SKIP decisions during scanning. (larsh: rev 464e7ce685486e3ede13ec2351b45b0a0b65696c) hbase-client/src/main/java/org/apache/hadoop/hbase/client/Scan.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java hbase-server/src/main/java/org/apache/hadoop/hbase/io/HalfStoreFileReader.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Committed to 0.98, 1.0.1, 1.1.0, and 2.0.0.
          The 0.98, and 1.0.1 mark Scan.HINT_LOOKAHEAD as deprecated, but the hint is no longer honored.
          From 1.1.0 the hint is gone.

          Show
          lhofhansl Lars Hofhansl added a comment - Committed to 0.98, 1.0.1, 1.1.0, and 2.0.0. The 0.98, and 1.0.1 mark Scan.HINT_LOOKAHEAD as deprecated, but the hint is no longer honored. From 1.1.0 the hint is gone.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          +1

          Show
          yuzhihong@gmail.com Ted Yu added a comment - +1
          Hide
          lhofhansl Lars Hofhansl added a comment -

          So in 0.98.x and 1.0.x I am going to deprecate Scan.HINT_LOOKAHEAD, and then remove it in 1.1.0 and 2.0.0.

          Even in 0.98 and 1.0 it'll be a no-op, just there to avoid breaking the API.

          The -0.98 patch here would go into 0.98. For 1.0.1 and 1.1.0 I'll see how nicely the trunk patch applies (it might need a version that's closer to the 0.98 version).

          I'll do that tomorrow.

          Show
          lhofhansl Lars Hofhansl added a comment - So in 0.98.x and 1.0.x I am going to deprecate Scan.HINT_LOOKAHEAD, and then remove it in 1.1.0 and 2.0.0. Even in 0.98 and 1.0 it'll be a no-op, just there to avoid breaking the API. The -0.98 patch here would go into 0.98. For 1.0.1 and 1.1.0 I'll see how nicely the trunk patch applies (it might need a version that's closer to the 0.98 version). I'll do that tomorrow.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          +1 on patch. Nice work!!

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - +1 on patch. Nice work!!
          Hide
          jamestaylor James Taylor added a comment -

          Awesome - nice work, Lars Hofhansl!

          Show
          jamestaylor James Taylor added a comment - Awesome - nice work, Lars Hofhansl !
          Hide
          lhofhansl Lars Hofhansl added a comment - - edited

          Same, but with each column in its own CF (in this case Phoenix does not use its WildcardTracker + Filter optimization)

          6CF case:

            q1 q2
          w/o patch 15.3 15.5
          w/ patch 9.14 9.19

          Any objection committing this to all branches?
          (stack, Andrew Purtell, ramkrishna.s.vasudevan?)

          James Taylor, FYI (we can probably remove the ColumnProjectionFilter optimization when this is in)

          Show
          lhofhansl Lars Hofhansl added a comment - - edited Same, but with each column in its own CF (in this case Phoenix does not use its WildcardTracker + Filter optimization) 6CF case:   q1 q2 w/o patch 15.3 15.5 w/ patch 9.14 9.19 Any objection committing this to all branches? ( stack , Andrew Purtell , ramkrishna.s.vasudevan ?) James Taylor , FYI (we can probably remove the ColumnProjectionFilter optimization when this is in)
          Hide
          lhofhansl Lars Hofhansl added a comment - - edited

          Did some more tests with Phoenix against 0.98, including some of the tests they used to validate their optimization to always use the WildcardColumnMatcher and doing the filtering themselves to avoid the cost of the ExplicitColumnTracker that does the seeking. Testing with 7 columns. One scenario was with all 7 columns in the same CF the other each column in its column family:

          Ran two queries: q1 = select count(1) where v3 = <> and v5 = <> and q2 = select avg(v2) where v3 = <> and v5 = <>

          1CF case:

            q1 w/o Phoenix p[t q1 w/ Phoenix opt q2 w/o Phoenix opt q2 w/ Phoenix opt
          w/o patch 12.9 8.4 18.0 8.3
          w/ patch 7.5 7.2 7.5 7.1

          Two observation:

          1. Even with the Phoenix optimization this is faster because a bunch of SEEK_NEXT_ROWs are saved unless they're necessary.
          2. The whole optimization is unnecessary now, it saves less than 5% in the best case with only one version per cell
          Show
          lhofhansl Lars Hofhansl added a comment - - edited Did some more tests with Phoenix against 0.98, including some of the tests they used to validate their optimization to always use the WildcardColumnMatcher and doing the filtering themselves to avoid the cost of the ExplicitColumnTracker that does the seeking. Testing with 7 columns. One scenario was with all 7 columns in the same CF the other each column in its column family: Ran two queries: q1 = select count(1) where v3 = <> and v5 = <> and q2 = select avg(v2) where v3 = <> and v5 = <> 1CF case:   q1 w/o Phoenix p[t q1 w/ Phoenix opt q2 w/o Phoenix opt q2 w/ Phoenix opt w/o patch 12.9 8.4 18.0 8.3 w/ patch 7.5 7.2 7.5 7.1 Two observation: Even with the Phoenix optimization this is faster because a bunch of SEEK_NEXT_ROWs are saved unless they're necessary. The whole optimization is unnecessary now, it saves less than 5% in the best case with only one version per cell
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Actually PE only write single column rows, this needs many columns (or deletes) to show any improvement.

          Show
          lhofhansl Lars Hofhansl added a comment - Actually PE only write single column rows, this needs many columns (or deletes) to show any improvement.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          This would most show with: (1) filtering on the server and (2) doing some seeking (i.e. selecting only some of the columns - so that the explicit tracker is used, deleting some rows - deleteColumns, etc)... All with many versions, few versions, etc.

          I'll run some perf eval as well (with the 0.98 patch, though).

          Quite exited about this. After many, many attempts... Finally a way to significantly reduce the cost of SEEKs when there aren't many versions or columns (and when there are many verions/columns we still SEEK).

          Show
          lhofhansl Lars Hofhansl added a comment - This would most show with: (1) filtering on the server and (2) doing some seeking (i.e. selecting only some of the columns - so that the explicit tracker is used, deleting some rows - deleteColumns, etc)... All with many versions, few versions, etc. I'll run some perf eval as well (with the 0.98 patch, though). Quite exited about this. After many, many attempts... Finally a way to significantly reduce the cost of SEEKs when there aren't many versions or columns (and when there are many verions/columns we still SEEK).
          Hide
          stack stack added a comment -

          Lars Hofhansl if you describe test you'd like I can try it here.

          Show
          stack stack added a comment - Lars Hofhansl if you describe test you'd like I can try it here.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12702102/13109-trunk-v5.txt
          against master branch at commit daed00fc98167870463e77b620e9adb6ce9b204d.
          ATTACHMENT ID: 12702102

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

          +1 tests included. The patch appears to include 9 new or modified tests.
          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          -1 checkstyle. The applied patch generated 1937 checkstyle errors (more than the master's current 1936 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 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/13055//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12702102/13109-trunk-v5.txt against master branch at commit daed00fc98167870463e77b620e9adb6ce9b204d. ATTACHMENT ID: 12702102 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 9 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 5 warning messages. -1 checkstyle . The applied patch generated 1937 checkstyle errors (more than the master's current 1936 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 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/13055//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13055//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          The optimize() logic makes sense. I think particularly it is going to be useful when there is one version of a cell and the filter/trackers say SEEK_TO_ROW/COL.
          Changing the HFileBlock Index to Cell is fine unless you have a concern on the number of objects being created and thrown away. In that case we may have to have a different approach but for the case of NO_INDEX_KEY-we cannot go with '==' check. In this case that is not there.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - The optimize() logic makes sense. I think particularly it is going to be useful when there is one version of a cell and the filter/trackers say SEEK_TO_ROW/COL. Changing the HFileBlock Index to Cell is fine unless you have a concern on the number of objects being created and thrown away. In that case we may have to have a different approach but for the case of NO_INDEX_KEY-we cannot go with '==' check. In this case that is not there.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Oh and saw your sample patch. I think my version is even more radical... I change indexed key to Cell everywhere above the HFileBlockIndex

          Show
          lhofhansl Lars Hofhansl added a comment - Oh and saw your sample patch. I think my version is even more radical... I change indexed key to Cell everywhere above the HFileBlockIndex
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Test run for -v5.

          Show
          lhofhansl Lars Hofhansl added a comment - Test run for -v5.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          TestBlockScanned passed locally.

          Show
          lhofhansl Lars Hofhansl added a comment - TestBlockScanned passed locally.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Ah. Too late, made a patch already And it does make things nicer. I'm fine with either -v4 or -v5.

          Show
          lhofhansl Lars Hofhansl added a comment - Ah. Too late, made a patch already And it does make things nicer. I'm fine with either -v4 or -v5.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Here's a version that make a Cell all the down at the block index. Index we only do that when we actually load a block this is OK performance wise.

          (I do not think I want to backport the KeyOnlyKeyValue portions to 0.98, so if we want a 0.98 patch I'd prefer to leave it the way it is)

          Lemme know what you think ramkrishna.s.vasudevan.

          Show
          lhofhansl Lars Hofhansl added a comment - Here's a version that make a Cell all the down at the block index. Index we only do that when we actually load a block this is OK performance wise. (I do not think I want to backport the KeyOnlyKeyValue portions to 0.98, so if we want a 0.98 patch I'd prefer to leave it the way it is) Lemme know what you think ramkrishna.s.vasudevan .
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12702089/nextIndexKVChange_new.patch
          against master branch at commit daed00fc98167870463e77b620e9adb6ce9b204d.
          ATTACHMENT ID: 12702089

          +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 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

          +1 javac. The applied patch does not increase the total number of javac 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 site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestBlocksScanned

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.camel.test.junit4.CamelTestSupport.doStopCamelContext(CamelTestSupport.java:450)
          at org.apache.camel.test.junit4.CamelTestSupport.tearDown(CamelTestSupport.java:351)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12702089/nextIndexKVChange_new.patch against master branch at commit daed00fc98167870463e77b620e9adb6ce9b204d. ATTACHMENT ID: 12702089 +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 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac 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 site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestBlocksScanned -1 core zombie tests . There are 1 zombie test(s): at org.apache.camel.test.junit4.CamelTestSupport.doStopCamelContext(CamelTestSupport.java:450) at org.apache.camel.test.junit4.CamelTestSupport.tearDown(CamelTestSupport.java:351) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13052//console This message is automatically generated.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          ut KeyValue.KVComparator.compareOnlyKeyPortion(Cell, Cell) will not work, because I cannot make a Cell from the seek Cell in SQM without materializing the byte[]... That's the part I have to avoid.

          I understand this as to why you have to avoid because you will not use the Cell from SQM directly as the ts and type is the one that you will be passing.
          We tried out a way in our internal branch in such cases where we want the FirstOnRow, LastOnCol, firstOnCol type of Kvs for which we created a new FirstOnCol cell object passing the cell - but the getTS and getType would return LATEST_TIMESTAMP/MIN_TIMESTAMP and type as MAX/MIN based on what we want such that it is two cells. Anyway I think changing to cell for nextIndexKey does not matter except that there is a new compare() API. Fine with carrying on as it is now in your patches. Thanks Lars.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - ut KeyValue.KVComparator.compareOnlyKeyPortion(Cell, Cell) will not work, because I cannot make a Cell from the seek Cell in SQM without materializing the byte[]... That's the part I have to avoid. I understand this as to why you have to avoid because you will not use the Cell from SQM directly as the ts and type is the one that you will be passing. We tried out a way in our internal branch in such cases where we want the FirstOnRow, LastOnCol, firstOnCol type of Kvs for which we created a new FirstOnCol cell object passing the cell - but the getTS and getType would return LATEST_TIMESTAMP/MIN_TIMESTAMP and type as MAX/MIN based on what we want such that it is two cells. Anyway I think changing to cell for nextIndexKey does not matter except that there is a new compare() API. Fine with carrying on as it is now in your patches. Thanks Lars.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Here's the 0.98 patch btw.

          Show
          lhofhansl Lars Hofhansl added a comment - Here's the 0.98 patch btw.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          NP ramkrishna.s.vasudevan. Thanks for taking a look!

          I am not actually too concerned about the KeyOnlyKeyValueObject (actually, could you have a look at the first patch I attached here, where I optimized it a bit?)
          I can make a Cell from the indexed key (in trunk at least). But KeyValue.KVComparator.compareOnlyKeyPortion(Cell, Cell) will not work, because I cannot make a Cell from the seek Cell in SQM without materializing the byte[]... That's the part I have to avoid.

          What I could do (in trunk) is... Instead of:

          public int compareKey(byte[] key, int koff, int klen,
              byte[] row, int roff, int rlen,
              byte[] fam, int foff, int flen,
              byte[] col, int coff, int clen,
              long ts, byte type)
          

          We'd wrap the indexed key in a KeyOnlyKeyValue and have:

          public int compareKey(Cell cell,
              byte[] row, int roff, int rlen,
              byte[] fam, int foff, int flen,
              byte[] col, int coff, int clen,
              long ts, byte type)
          

          I actually think then we should do it all the way down at AbstractHFileScanner and store the nextIndexedKey as Cell instead of byte[].

          Lemme do that.

          Show
          lhofhansl Lars Hofhansl added a comment - NP ramkrishna.s.vasudevan . Thanks for taking a look! I am not actually too concerned about the KeyOnlyKeyValueObject (actually, could you have a look at the first patch I attached here, where I optimized it a bit?) I can make a Cell from the indexed key (in trunk at least). But KeyValue.KVComparator.compareOnlyKeyPortion(Cell, Cell) will not work, because I cannot make a Cell from the seek Cell in SQM without materializing the byte[]... That's the part I have to avoid. What I could do (in trunk) is... Instead of: public int compareKey( byte [] key, int koff, int klen, byte [] row, int roff, int rlen, byte [] fam, int foff, int flen, byte [] col, int coff, int clen, long ts, byte type) We'd wrap the indexed key in a KeyOnlyKeyValue and have: public int compareKey(Cell cell, byte [] row, int roff, int rlen, byte [] fam, int foff, int flen, byte [] col, int coff, int clen, long ts, byte type) I actually think then we should do it all the way down at AbstractHFileScanner and store the nextIndexedKey as Cell instead of byte[]. Lemme do that.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -
          this.nextIndexedKV == HConstants.NO_NEXT_INDEXED_KV
          

          This change in the patch is not quite right. Sorry about that. Here we may have to do a compare only if we change to Cell.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - this .nextIndexedKV == HConstants.NO_NEXT_INDEXED_KV This change in the patch is not quite right. Sorry about that. Here we may have to do a compare only if we change to Cell.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          Am not hijacking the issue. Just trying to convey my point here.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - Am not hijacking the issue. Just trying to convey my point here.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          larsh
          I agree on the fact that creating KeyOnlyKeyValue objects is going to have some cost. Anyway that is not going to copy the contents of the KV. Only going to wrap it. As in your first patch we were copying the key part so that would have been costlier.
          But still we have the option of setting the key part to the KeyOnlyKeyValue object. So am just attaching a patch to change nextIndexKey to a Cell using KeyOnlyKeyValue. And just keep setting and resetting the nextIndexKey that comes from the HFileBlockIndex.
          The main reason I suggest t his way is that for the Offheap changes and BB related changes, if we have Cells it would be easier to do these compares rather than adding new type of comparators. Can you try to profile with this change and if you really see that KeyOnlyKV is affecting the performance then +1 on going ahead with pure byte[] based. What do you think. It is not a patch prepared over your patch, just a suggestion to change the nextIndexKey to Kv.

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - larsh I agree on the fact that creating KeyOnlyKeyValue objects is going to have some cost. Anyway that is not going to copy the contents of the KV. Only going to wrap it. As in your first patch we were copying the key part so that would have been costlier. But still we have the option of setting the key part to the KeyOnlyKeyValue object. So am just attaching a patch to change nextIndexKey to a Cell using KeyOnlyKeyValue. And just keep setting and resetting the nextIndexKey that comes from the HFileBlockIndex. The main reason I suggest t his way is that for the Offheap changes and BB related changes, if we have Cells it would be easier to do these compares rather than adding new type of comparators. Can you try to profile with this change and if you really see that KeyOnlyKV is affecting the performance then +1 on going ahead with pure byte[] based. What do you think. It is not a patch prepared over your patch, just a suggestion to change the nextIndexKey to Kv.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Ok... With deletes. Same as above but with an additional 400k deletes (deleted all columns every 10th row).

          Without patch:

          Wildcard Col 2+4
          4.38 12.0

          With patch:

          Wildcard Col 2+4
          4.39 4.74
          Show
          lhofhansl Lars Hofhansl added a comment - Ok... With deletes. Same as above but with an additional 400k deletes (deleted all columns every 10th row). Without patch: Wildcard Col 2+4 4.38 12.0 With patch: Wildcard Col 2+4 4.39 4.74
          Hide
          lhofhansl Lars Hofhansl added a comment -

          One more test I'll do is with deletes.

          Show
          lhofhansl Lars Hofhansl added a comment - One more test I'll do is with deletes.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Some more test with many HFiles (4m rows, 5 cols, 1 version - as above). No compactions at all -> 25 HFiles of 10mb each.

          Without patch:

          Wildcard Col 2+4
          4.59 13.5

          With patch:

          Wildcard Col 2+4
          4.38 4.89

          Almost a 3x improvement. I have convinced myself that this is good.

          Somebody up for independent tests? I have a 0.98 patch as well (in fact that's the one I have used for testing)?

          Show
          lhofhansl Lars Hofhansl added a comment - Some more test with many HFiles (4m rows, 5 cols, 1 version - as above). No compactions at all -> 25 HFiles of 10mb each. Without patch: Wildcard Col 2+4 4.59 13.5 With patch: Wildcard Col 2+4 4.38 4.89 Almost a 3x improvement. I have convinced myself that this is good. Somebody up for independent tests? I have a 0.98 patch as well (in fact that's the one I have used for testing)?
          Hide
          lhofhansl Lars Hofhansl added a comment -
          • left optimization on for gets
          • fixes TestBlocksRead

          Note that this test reads some more of the 1-byte blocks, which does not translate to real-life bad behavior. In this extreme case the logic decides to ignore the SEEK and do a SKIP instead, because it thinks the seek would get us to another block.

          I'll do some tests with many store files to confirm.

          Show
          lhofhansl Lars Hofhansl added a comment - left optimization on for gets fixes TestBlocksRead Note that this test reads some more of the 1-byte blocks, which does not translate to real-life bad behavior. In this extreme case the logic decides to ignore the SEEK and do a SKIP instead, because it thinks the seek would get us to another block. I'll do some tests with many store files to confirm.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          TestScanEarlyTermination passed locally.

          Also, thanks ramkrishna.s.vasudevan for looking at the comparator.
          And re: the KeyOnlyKeyValue, I saw that was done down in the AbstrackHFileScanner.reseekTo, there it's not time critical, since it only happens when we issued a seek anyway.

          Show
          lhofhansl Lars Hofhansl added a comment - TestScanEarlyTermination passed locally. Also, thanks ramkrishna.s.vasudevan for looking at the comparator. And re: the KeyOnlyKeyValue, I saw that was done down in the AbstrackHFileScanner.reseekTo, there it's not time critical, since it only happens when we issued a seek anyway.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          make the nextIndexedKEy to a Cell and create a KeyOnlyKeyValue out of it. Then use the normal CellComparator.compare(cell, cell).

          I agree that would be nicer. But that would be very slow. This decision is made for every single KeyValue (if the SQM decides that it wants to seek). That's why I added a special compare that the key can be compared in place without creating a KV object or even a new byte[].

          Note that there would be two Cell to be created: (1) the Cell representing the indexed key, and (2) the Cell representing the seek key in the SQM. With this patch no new objects are created at all.

          Just avoiding the creating the key array saved 0.7s over 4m rows (5 cols). Making KeyValues or Cells would be more expensive.

          Show
          lhofhansl Lars Hofhansl added a comment - make the nextIndexedKEy to a Cell and create a KeyOnlyKeyValue out of it. Then use the normal CellComparator.compare(cell, cell). I agree that would be nicer. But that would be very slow. This decision is made for every single KeyValue (if the SQM decides that it wants to seek). That's why I added a special compare that the key can be compared in place without creating a KV object or even a new byte[]. Note that there would be two Cell to be created: (1) the Cell representing the indexed key, and (2) the Cell representing the seek key in the SQM. With this patch no new objects are created at all. Just avoiding the creating the key array saved 0.7s over 4m rows (5 cols). Making KeyValues or Cells would be more expensive.
          Hide
          ram_krish ramkrishna.s.vasudevan added a comment -

          larsh
          Still going thro the logic of the patch. I verified the comparator API that was added. Seems fine to me.
          But one suggestion I have is, make the nextIndexedKEy to a Cell and create a KeyOnlyKeyValue out of it. Then use the normal CellComparator.compare(cell, cell). Will that not work out. What do you think?

          Show
          ram_krish ramkrishna.s.vasudevan added a comment - larsh Still going thro the logic of the patch. I verified the comparator API that was added. Seems fine to me. But one suggestion I have is, make the nextIndexedKEy to a Cell and create a KeyOnlyKeyValue out of it. Then use the normal CellComparator.compare(cell, cell). Will that not work out. What do you think?
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12701810/13109-trunk-v3.txt
          against master branch at commit 4fb6f91cbad7d9b3c18f897ee3a4f70dc7c21595.
          ATTACHMENT ID: 12701810

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

          +1 tests included. The patch appears to include 6 new or modified tests.
          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.security.access.TestScanEarlyTermination

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12701810/13109-trunk-v3.txt against master branch at commit 4fb6f91cbad7d9b3c18f897ee3a4f70dc7c21595. ATTACHMENT ID: 12701810 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.security.access.TestScanEarlyTermination Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13032//console This message is automatically generated.
          Hide
          lhofhansl Lars Hofhansl added a comment -
          • passes test
          • disabled optimization for Gets
          • removed commented code
          • minor cleanups
          Show
          lhofhansl Lars Hofhansl added a comment - passes test disabled optimization for Gets removed commented code minor cleanups
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Should deprecate Scan.LOOK_AHEAD in 1.0.1, so that we can remove it in 1.1. (per our policy that is possible)

          The indexed key comes out of the HFile as a key - and yes it presumes a KeyValue-key all over the place.
          Translating this into a Cell would be measurably slower, could try to record it as Cell in the first place.

          The compare in KV is needed unfortunately to avoid materializing the seek key just for this check. I did not like to write that part.

          Yeah need to remove commented stuff.

          Optimize is optimizing heuristically.

          • many versions of KVs are spread all over the HFiles. The heuristic of checking the top scanner might not be optimal in that case. But then too, we'd need to seek into many files for the reset, so compared the cost should be low.
          • SQM says SEEK, and optimize does not change this. In that case we wasted a compare, that's OK, seek is way more expensive.
          • It is a heuristic. In some one off cases we might be doing some SKIP before we end up seeking.

          I'd not be afraid to deploy for us in production (I am most worried that I got the new compare method wrong... Any chance eyeballing that stack?)

          New patch coming to fix the test. The test is weird, setting the block size to 1 (yes, 1 byte), and then it counts the blocks loaded for Bloom filters - of course this throws this off. I will disable this optimization for Gets anyway, there's no point.

          Show
          lhofhansl Lars Hofhansl added a comment - Should deprecate Scan.LOOK_AHEAD in 1.0.1, so that we can remove it in 1.1. (per our policy that is possible) The indexed key comes out of the HFile as a key - and yes it presumes a KeyValue-key all over the place. Translating this into a Cell would be measurably slower, could try to record it as Cell in the first place. The compare in KV is needed unfortunately to avoid materializing the seek key just for this check. I did not like to write that part. Yeah need to remove commented stuff. Optimize is optimizing heuristically. many versions of KVs are spread all over the HFiles. The heuristic of checking the top scanner might not be optimal in that case. But then too, we'd need to seek into many files for the reset, so compared the cost should be low. SQM says SEEK, and optimize does not change this. In that case we wasted a compare, that's OK, seek is way more expensive. It is a heuristic. In some one off cases we might be doing some SKIP before we end up seeking. I'd not be afraid to deploy for us in production (I am most worried that I got the new compare method wrong... Any chance eyeballing that stack ?) New patch coming to fix the test. The test is weird, setting the block size to 1 (yes, 1 byte), and then it counts the blocks loaded for Bloom filters - of course this throws this off. I will disable this optimization for Gets anyway, there's no point.
          Hide
          stack stack added a comment -

          Should Scan.LOOK_AHEAD be deprecated/become a noop in case someone using it?

          We need to add more compare to KV? There ain't enough going on in there already (smile)?

          getNextIndexedKey makes sense but should we be returning byte [] ? Why not Cell? byte [] presumes a certain format?

          getKeyForNextRow is commented out. Remove?

          I like the way you add in this optimize method and it works or it doesn't.

          When will optimize be optimal? When will it not add value ( you say selecting 2 and 4 in above is worse case but generally?) Sorry for dumb questions. I don't know this stuff well.

          Show
          stack stack added a comment - Should Scan.LOOK_AHEAD be deprecated/become a noop in case someone using it? We need to add more compare to KV? There ain't enough going on in there already (smile)? getNextIndexedKey makes sense but should we be returning byte [] ? Why not Cell? byte [] presumes a certain format? getKeyForNextRow is commented out. Remove? I like the way you add in this optimize method and it works or it doesn't. When will optimize be optimal? When will it not add value ( you say selecting 2 and 4 in above is worse case but generally?) Sorry for dumb questions. I don't know this stuff well.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12701779/13109-trunk-v2.txt
          against master branch at commit 70ecf18817ef219389a9e024ff21ffb99b6615d9.
          ATTACHMENT ID: 12701779

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

          +1 tests included. The patch appears to include 6 new or modified tests.
          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestBlocksRead

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12701779/13109-trunk-v2.txt against master branch at commit 70ecf18817ef219389a9e024ff21ffb99b6615d9. ATTACHMENT ID: 12701779 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestBlocksRead Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13030//console This message is automatically generated.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Numbers with new patch (avoiding the array creation helps):
          With patch:

          Wildcard Col 2+4
          3.9 4.4

          The ExplicitColumnTracker is now only 10% slower than the wildcard column tracker (was almost 2x before).

          Show
          lhofhansl Lars Hofhansl added a comment - Numbers with new patch (avoiding the array creation helps): With patch: Wildcard Col 2+4 3.9 4.4 The ExplicitColumnTracker is now only 10% slower than the wildcard column tracker (was almost 2x before).
          Hide
          lhofhansl Lars Hofhansl added a comment -

          New patch.

          1. fixes the test failures
          2. avoids the array creation for comparison (compares the indexed key with the next key in place)
          Show
          lhofhansl Lars Hofhansl added a comment - New patch. fixes the test failures avoids the array creation for comparison (compares the indexed key with the next key in place)
          Hide
          lhofhansl Lars Hofhansl added a comment -

          All the test failures have the same cause (using KeyOnlyKeyValue), I removed that part of the code (was just to avoid some code duplication).

          Show
          lhofhansl Lars Hofhansl added a comment - All the test failures have the same cause (using KeyOnlyKeyValue), I removed that part of the code (was just to avoid some code duplication).
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Will look at those tomorrow.

          Show
          lhofhansl Lars Hofhansl added a comment - Will look at those tomorrow.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12701657/13109-trunk.txt
          against master branch at commit dd78f459e8f10e4587742a049e38d8c6b50dd0cb.
          ATTACHMENT ID: 12701657

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

          +1 tests included. The patch appears to include 6 new or modified tests.
          +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestScanWithBloomError
          org.apache.hadoop.hbase.regionserver.TestScanner
          org.apache.hadoop.hbase.regionserver.TestStoreFileRefresherChore
          org.apache.hadoop.hbase.regionserver.TestColumnSeeking
          org.apache.hadoop.hbase.regionserver.TestMinVersions
          org.apache.hadoop.hbase.regionserver.TestResettingCounters

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/checkstyle-aggregate.html

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12701657/13109-trunk.txt against master branch at commit dd78f459e8f10e4587742a049e38d8c6b50dd0cb. ATTACHMENT ID: 12701657 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1937 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 site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestScanWithBloomError org.apache.hadoop.hbase.regionserver.TestScanner org.apache.hadoop.hbase.regionserver.TestStoreFileRefresherChore org.apache.hadoop.hbase.regionserver.TestColumnSeeking org.apache.hadoop.hbase.regionserver.TestMinVersions org.apache.hadoop.hbase.regionserver.TestResettingCounters Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/checkstyle-aggregate.html Javadoc warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//artifact/patchprocess/patchJavadocWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13020//console This message is automatically generated.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          (I might work on optimizing requiring to call matcher.getKeyForNextColumn, since that produces a new byte[] that's I'd rather avoid)

          Show
          lhofhansl Lars Hofhansl added a comment - (I might work on optimizing requiring to call matcher.getKeyForNextColumn, since that produces a new byte[] that's I'd rather avoid)
          Hide
          lhofhansl Lars Hofhansl added a comment -

          I'll stop here until get some feedback. Any thoughts (stack?)

          Show
          lhofhansl Lars Hofhansl added a comment - I'll stop here until get some feedback. Any thoughts ( stack ?)
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Let's get a QA run in.

          Show
          lhofhansl Lars Hofhansl added a comment - Let's get a QA run in.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Some more tests (similar to those in HBASE-9778, but this a different machine so don't compare them in absolute values): 4m row, 5 cols, 1 version.

          Without patch:

          Wildcard Col 2+4
          3.9 7.27

          With patch:

          Wildcard Col 2+4
          3.9 5.1

          (selecting columns 2 and 4 is the worst case)

          So this patch improves the ExplicitColumnTracker by almost 1/3rd, and the beauty of this change is that it will still work with very many versions, because it uses whether we can seek into another block as a metric to decide whether to seek or not.

          Show
          lhofhansl Lars Hofhansl added a comment - Some more tests (similar to those in HBASE-9778 , but this a different machine so don't compare them in absolute values): 4m row, 5 cols, 1 version. Without patch: Wildcard Col 2+4 3.9 7.27 With patch: Wildcard Col 2+4 3.9 5.1 (selecting columns 2 and 4 is the worst case) So this patch improves the ExplicitColumnTracker by almost 1/3rd, and the beauty of this change is that it will still work with very many versions, because it uses whether we can seek into another block as a metric to decide whether to seek or not.
          Hide
          lhofhansl Lars Hofhansl added a comment -

          The first patch does a bunch of things:

          1. get rid of Scan.LOOK_AHEAD, that is no longer needed
          2. Optimize KeyValue.KeyOnlyKeyValue a bit (to reuse the fields from KeyValue and save a few bytes of heap) - that part I could remove, I'm not using that anymore, but it seemed good anyway.
          3. Uses the next indexed key that the HFileScanner already maintain to decide whether a seek would be likely to seek to a new block. If so the StoreScanner will continue to issue SEEKs, otherwise it will SKIP instead.
          4. Adds a some helpers to KeyValueUtil to build a key array to avoid creating KeyValue objects unnecessarily
          Show
          lhofhansl Lars Hofhansl added a comment - The first patch does a bunch of things: get rid of Scan.LOOK_AHEAD, that is no longer needed Optimize KeyValue.KeyOnlyKeyValue a bit (to reuse the fields from KeyValue and save a few bytes of heap) - that part I could remove, I'm not using that anymore, but it seemed good anyway. Uses the next indexed key that the HFileScanner already maintain to decide whether a seek would be likely to seek to a new block. If so the StoreScanner will continue to issue SEEKs, otherwise it will SKIP instead. Adds a some helpers to KeyValueUtil to build a key array to avoid creating KeyValue objects unnecessarily
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Here's the patch from HBASE-12311.

          Show
          lhofhansl Lars Hofhansl added a comment - Here's the patch from HBASE-12311 .
          Hide
          lhofhansl Lars Hofhansl added a comment -

          Doesn't quite work since we check the timeranges before the individual columns (and so the column trackers have no way to manage their state - i.e. track when the current column is done)

          Show
          lhofhansl Lars Hofhansl added a comment - Doesn't quite work since we check the timeranges before the individual columns (and so the column trackers have no way to manage their state - i.e. track when the current column is done)
          Hide
          lhofhansl Lars Hofhansl added a comment -

          0.98 patch. Untested, I only checked that it compiles. Just parking.

          Show
          lhofhansl Lars Hofhansl added a comment - 0.98 patch. Untested, I only checked that it compiles. Just parking.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development