You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2021/06/29 02:34:15 UTC

[GitHub] [hbase] jojochuang opened a new pull request #3434: HBASE-21946. Use hasCapabilities() to detect and switch pread API accordingly.

jojochuang opened a new pull request #3434:
URL: https://github.com/apache/hbase/pull/3434


   Use StreamCapabilities.hasCapabilities() to determine if the pread is supported by the input stream. Use reflection to invoke the bytebuffer pread method when it is available (because the ByteBufferPositionedReadable API is not available until Hadoop 3.3). This way, we can maintain backward compatibility for any Hadoop version above 2.9.
   
   Passed a few unit tests building with Hadoop 3.3.1 and 3.1.2. Open this PR to run through the whole precommit test.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] jojochuang commented on a change in pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#discussion_r660361583



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/BlockIOUtils.java
##########
@@ -207,6 +234,17 @@ public static boolean readWithExtra(ByteBuff buf, FSDataInputStream dis, int nec
    */
   public static boolean preadWithExtra(ByteBuff buff, FSDataInputStream dis, long position,
       int necessaryLen, int extraLen) throws IOException {
+    boolean preadbytebuffer = dis.hasCapability("in:preadbytebuffer");

Review comment:
       I suppose the overhead is relatively low. I had thought about moving to where the input stream is created, but it would require pretty extensive refactoring.
   
   Internally, hasCapability() uses switch statement to compare the capability string and it should be quite efficient.
   
   I can explore this suggestion later.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-885862500


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 48s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 10s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 28s |  master passed  |
   | +1 :green_heart: |  shadedjars  |  10m 53s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 12s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 49s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 49s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |  10m 46s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  9s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 59s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 152m  6s |  hbase-server in the patch passed.  |
   |  |   | 194m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 093dbe96d8b1 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 51ed95c0cb |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/testReport/ |
   | Max. process+thread count | 3732 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-885839974


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 26s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 25s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 39s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  9s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 42s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 42s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m  7s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m  7s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 138m 55s |  hbase-server in the patch passed.  |
   |  |   | 175m  4s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux adcb2c0eaf1f 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 51ed95c0cb |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/testReport/ |
   | Max. process+thread count | 4129 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-885795575


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 31s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 29s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   7m 51s |  master passed  |
   | +1 :green_heart: |  compile  |   7m 54s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 53s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   4m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 20s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   6m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   7m  1s |  the patch passed  |
   | +1 :green_heart: |  javac  |   7m  1s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   2m 21s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  31m 19s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   5m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 47s |  The patch does not generate ASF License warnings.  |
   |  |   |  92m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 1c81363c6ab1 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 51ed95c0cb |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/2/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-870201695


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 54s |  master passed  |
   | +1 :green_heart: |  compile  |   3m 57s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 27s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 45s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 23s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 13s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 27s |  The patch does not generate ASF License warnings.  |
   |  |   |  52m  2s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 992c1cda610a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 22ec681ad9 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-870245104


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 55s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 25s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 42s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 23s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 48s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 131m 11s |  hbase-server in the patch passed.  |
   |  |   | 165m 34s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux a08622e1855f 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 22ec681ad9 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/testReport/ |
   | Max. process+thread count | 3640 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#discussion_r660287517



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/BlockIOUtils.java
##########
@@ -207,6 +234,17 @@ public static boolean readWithExtra(ByteBuff buf, FSDataInputStream dis, int nec
    */
   public static boolean preadWithExtra(ByteBuff buff, FSDataInputStream dis, long position,
       int necessaryLen, int extraLen) throws IOException {
+    boolean preadbytebuffer = dis.hasCapability("in:preadbytebuffer");

Review comment:
       We need to test it every time? Is it possible to test once when the FSDataInputStream is created?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-870201695






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] jojochuang commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
jojochuang commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-886537151


   Thanks @Apache9 . Took me a few days because I initially thought it's either impossible or requires lots of dirty hack to add a UT. But turns out it's quite straightforward. I'll merge later.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#issuecomment-870247452


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   6m 52s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 37s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 38s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m  2s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 128m 37s |  hbase-server in the patch passed.  |
   |  |   | 171m 11s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3434 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7598c6f678d9 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 22ec681ad9 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/testReport/ |
   | Max. process+thread count | 3407 (vs. ulimit of 30000) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3434/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#discussion_r660287517



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/BlockIOUtils.java
##########
@@ -207,6 +234,17 @@ public static boolean readWithExtra(ByteBuff buf, FSDataInputStream dis, int nec
    */
   public static boolean preadWithExtra(ByteBuff buff, FSDataInputStream dis, long position,
       int necessaryLen, int extraLen) throws IOException {
+    boolean preadbytebuffer = dis.hasCapability("in:preadbytebuffer");

Review comment:
       We need to test it every time? Is it possible to test once when the FSDataInputStream is created?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] jojochuang commented on a change in pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #3434:
URL: https://github.com/apache/hbase/pull/3434#discussion_r660361583



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/BlockIOUtils.java
##########
@@ -207,6 +234,17 @@ public static boolean readWithExtra(ByteBuff buf, FSDataInputStream dis, int nec
    */
   public static boolean preadWithExtra(ByteBuff buff, FSDataInputStream dis, long position,
       int necessaryLen, int extraLen) throws IOException {
+    boolean preadbytebuffer = dis.hasCapability("in:preadbytebuffer");

Review comment:
       I suppose the overhead is relatively low. I had thought about moving to where the input stream is created, but it would require pretty extensive refactoring.
   
   Internally, hasCapability() uses switch statement to compare the capability string and it should be quite efficient.
   
   I can explore this suggestion later.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] jojochuang merged pull request #3434: HBASE-21946 Use ByteBuffer pread instead of byte[] pread in HFileBlock when applicable

Posted by GitBox <gi...@apache.org>.
jojochuang merged pull request #3434:
URL: https://github.com/apache/hbase/pull/3434


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org