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 2022/07/21 12:38:03 UTC
[GitHub] [hbase] wchevreuil opened a new pull request, #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
wchevreuil opened a new pull request, #4640:
URL: https://github.com/apache/hbase/pull/4640
…k should be closed
--
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] bbeaudreault commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926654635
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
Review Comment:
so this changes the default behavior i believe. do you think this change is applicable to most users (i.e. net positive for them)? not against it, just asking... alternatively we can add handling of 0 value below and leave default. i have no opinion since i dont fully grasp the feature yet.
--
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 diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r927756004
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:
##########
@@ -449,7 +449,7 @@ public int getOnDiskSizeWithHeader() {
}
/** Returns the on-disk size of the data part + checksum (header excluded). */
- int getOnDiskSizeWithoutHeader() {
+ public int getOnDiskSizeWithoutHeader() {
Review Comment:
Why do we need to change this modifier? Seems we do not call in the newly added tests?
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192570416
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 11s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 35s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | the patch passed |
| +1 :green_heart: | compile | 0m 45s | the patch passed |
| +1 :green_heart: | javac | 0m 45s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 41s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 205m 35s | hbase-server in the patch passed. |
| | | 223m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5829efe74866 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/testReport/ |
| Max. process+thread count | 2623 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/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] bbeaudreault commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926649410
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
+ this.encodedBlockSizeLimit = encodeBlockSizeRatio >0 ?
Review Comment:
seems like you could leave this as is, since multiplying blocksize by 0 would still be 0. not a huge deal
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192721010
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 6s | master passed |
| +1 :green_heart: | compile | 2m 15s | master passed |
| +1 :green_heart: | checkstyle | 0m 27s | master passed |
| +1 :green_heart: | spotless | 0m 41s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 13s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 5s | the patch passed |
| +1 :green_heart: | compile | 2m 11s | the patch passed |
| +1 :green_heart: | javac | 2m 11s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 27s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 24s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 30m 38s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 57d92a87ab75 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 64 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192884094
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 43s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 2s | master passed |
| +1 :green_heart: | compile | 2m 10s | master passed |
| +1 :green_heart: | checkstyle | 0m 27s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 11s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 4s | the patch passed |
| +1 :green_heart: | compile | 2m 11s | the patch passed |
| +1 :green_heart: | javac | 2m 11s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 26s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 30m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 7606559bb71f 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 66 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/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] wchevreuil merged pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil merged PR #4640:
URL: https://github.com/apache/hbase/pull/4640
--
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] bbeaudreault commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926652309
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -309,12 +311,15 @@ protected void finishInit(final Configuration conf) {
* At a block boundary, write all the inline blocks and opens new block.
*/
protected void checkBlockBoundary() throws IOException {
- // For encoder like prefixTree, encoded size is not available, so we have to compare both
- // encoded size and unencoded size to blocksize limit.
- if (
- blockWriter.encodedBlockSizeWritten() >= encodedBlockSizeLimit
- || blockWriter.blockSizeWritten() >= hFileContext.getBlocksize()
- ) {
+ boolean shouldFinishBlock = false;
+ //This means hbase.writer.unified.encoded.blocksize.ratio was set to something different from 0
Review Comment:
any implication on the comment there was here before which says we need to compare both? granted I think prefixTree has been removed
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192109742
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 26s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 3m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 8s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 54s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 199m 27s | hbase-server in the patch failed. |
| | | 215m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5b0c1cc1cf52 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/testReport/ |
| Max. process+thread count | 2568 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192019004
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 40s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 29s | master passed |
| +1 :green_heart: | compile | 2m 18s | master passed |
| +1 :green_heart: | checkstyle | 0m 29s | master passed |
| +1 :green_heart: | spotless | 0m 44s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 16s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 6s | the patch passed |
| +1 :green_heart: | compile | 2m 17s | the patch passed |
| +1 :green_heart: | javac | 2m 17s | the patch passed |
| -0 :warning: | checkstyle | 0m 29s | hbase-server: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 54s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. |
| +1 :green_heart: | spotless | 0m 42s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 32m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 97fd26bb0007 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 64 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r927827848
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:
##########
@@ -449,7 +449,7 @@ public int getOnDiskSizeWithHeader() {
}
/** Returns the on-disk size of the data part + checksum (header excluded). */
- int getOnDiskSizeWithoutHeader() {
+ public int getOnDiskSizeWithoutHeader() {
Review Comment:
Yes, not used on tests anymore. Removing...
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192112819
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 9s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 55s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 43s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 38s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 204m 17s | hbase-server in the patch failed. |
| | | 222m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0f7986c0f173 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/2/testReport/ |
| Max. process+thread count | 2993 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192859035
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 2s | master passed |
| +1 :green_heart: | compile | 0m 33s | master passed |
| +1 :green_heart: | shadedjars | 3m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 8s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 199m 47s | hbase-server in the patch passed. |
| | | 215m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 31e036821b65 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/testReport/ |
| Max. process+thread count | 2534 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926886208
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -309,12 +311,15 @@ protected void finishInit(final Configuration conf) {
* At a block boundary, write all the inline blocks and opens new block.
*/
protected void checkBlockBoundary() throws IOException {
- // For encoder like prefixTree, encoded size is not available, so we have to compare both
- // encoded size and unencoded size to blocksize limit.
- if (
- blockWriter.encodedBlockSizeWritten() >= encodedBlockSizeLimit
- || blockWriter.blockSizeWritten() >= hFileContext.getBlocksize()
- ) {
+ boolean shouldFinishBlock = false;
+ //This means hbase.writer.unified.encoded.blocksize.ratio was set to something different from 0
Review Comment:
Yeah, we don't have prefix tree anymore. So with the previous "||" condition, we could still not respect the encoded size desired if the data shrinkage by encoding is higher than the configured "hbase.writer.unified.encoded.blocksize.ratio' value. This change also allows for defining a 1:1 ration where we would then use the encoded size for the block limit.
--
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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926890128
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
Review Comment:
It doesn't change the default behaviour in the sense that if "hbase.writer.unified.encoded.blocksize.ratio" isn't set, we consider only the unencoded size for calculating block limit, which is the same with previous if condition on checkBlockBoundary method.
The difference is when "hbase.writer.unified.encoded.blocksize.ratio" is set, as we now can have 64KB of encoded data (whilst it would never be possible before).
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192562267
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 43s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 12s | master passed |
| +1 :green_heart: | compile | 0m 35s | master passed |
| +1 :green_heart: | shadedjars | 3m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 10s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 54s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 198m 56s | hbase-server in the patch passed. |
| | | 214m 49s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2d1227f83a14 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/testReport/ |
| Max. process+thread count | 2772 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192990113
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 10s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 35s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 47s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 40s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 41s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 202m 56s | hbase-server in the patch passed. |
| | | 220m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 70bc8fece001 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/testReport/ |
| Max. process+thread count | 2755 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1191469247
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 24s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 30s | master passed |
| +1 :green_heart: | compile | 2m 16s | master passed |
| +1 :green_heart: | checkstyle | 0m 29s | master passed |
| +1 :green_heart: | spotless | 0m 43s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 15s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 7s | the patch passed |
| +1 :green_heart: | compile | 2m 15s | the patch passed |
| +1 :green_heart: | javac | 2m 15s | the patch passed |
| -0 :warning: | checkstyle | 0m 27s | hbase-server: The patch generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 47s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. |
| -1 :x: | spotless | 0m 35s | patch has 65 errors when running spotless:check, run spotless:apply to fix. |
| +1 :green_heart: | spotbugs | 1m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 32m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 8b20caa9c393 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 64 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/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] anmolnar commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
anmolnar commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926641268
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java:
##########
@@ -1141,4 +1144,64 @@ public void testDataBlockEncodingMetaData() throws IOException {
byte[] value = fileInfo.get(HFileDataBlockEncoder.DATA_BLOCK_ENCODING);
assertArrayEquals(dataBlockEncoderAlgo.getNameInBytes(), value);
}
+
+ @Test
+ public void testDataBlockSizeEncoded() throws Exception {
+ // Make up a directory hierarchy that has a regiondir ("7e0102") and familyname.
+ Path dir = new Path(new Path(this.testDir, "7e0102"), "familyname");
+ Path path = new Path(dir, "1234567890");
+
+ DataBlockEncoding dataBlockEncoderAlgo =
+ DataBlockEncoding.FAST_DIFF;
+
+ conf.setDouble("hbase.writer.unified.encoded.blocksize.ratio", 1);
+
+ cacheConf = new CacheConfig(conf);
+ HFileContext meta = new HFileContextBuilder().withBlockSize(BLOCKSIZE_SMALL)
+ .withChecksumType(CKTYPE)
+ .withBytesPerCheckSum(CKBYTES)
+ .withDataBlockEncoding(dataBlockEncoderAlgo)
+ .build();
+ // Make a store file and write data to it.
+ StoreFileWriter writer = new StoreFileWriter.Builder(conf, cacheConf, this.fs)
+ .withFilePath(path)
+ .withMaxKeyCount(2000)
+ .withFileContext(meta)
+ .build();
+ writeStoreFile(writer);
+ //writer.close();
Review Comment:
Please remove this line.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
+ this.encodedBlockSizeLimit = encodeBlockSizeRatio >0 ?
+ (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio) : 0;
Review Comment:
Because of the multiplication the result will be 0 anyway, no need for the Elvis operator.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:
##########
@@ -449,7 +449,7 @@ public int getOnDiskSizeWithHeader() {
}
/** Returns the on-disk size of the data part + checksum (header excluded). */
- int getOnDiskSizeWithoutHeader() {
+ public int getOnDiskSizeWithoutHeader() {
Review Comment:
I'm not sure if it's feasible, but if you move the test to `TestHFile`, for instance, you don't need to make this method public.
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1191736286
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 28s | 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 _ |
| +1 :green_heart: | mvninstall | 4m 8s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 6m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 47s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 39s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 29s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 244m 3s | hbase-server in the patch failed. |
| | | 269m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a87f83f1853c 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/1/testReport/ |
| Max. process+thread count | 2562 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926879730
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
+ this.encodedBlockSizeLimit = encodeBlockSizeRatio >0 ?
Review Comment:
Yes.
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192411713
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 2s | master passed |
| +1 :green_heart: | compile | 2m 11s | master passed |
| +1 :green_heart: | checkstyle | 0m 26s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 10s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 0s | the patch passed |
| +1 :green_heart: | compile | 2m 8s | the patch passed |
| +1 :green_heart: | javac | 2m 8s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 21s | Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1. |
| -1 :x: | spotless | 0m 34s | patch has 24 errors when running spotless:check, run spotless:apply to fix. |
| +1 :green_heart: | spotbugs | 1m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 30m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux e8d2eeae86dc 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 64 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/3/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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926875844
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
+ this.encodedBlockSizeLimit = encodeBlockSizeRatio >0 ?
+ (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio) : 0;
Review Comment:
Yes.
--
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] wchevreuil commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926890128
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -172,8 +172,10 @@ public HFileWriterImpl(final Configuration conf, CacheConfig cacheConf, Path pat
}
closeOutputStream = path != null;
this.cacheConf = cacheConf;
- float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 1f);
- this.encodedBlockSizeLimit = (int) (hFileContext.getBlocksize() * encodeBlockSizeRatio);
+ float encodeBlockSizeRatio = conf.getFloat(UNIFIED_ENCODED_BLOCKSIZE_RATIO, 0f);
Review Comment:
It doesn't change the default behaviour in the sense that if "hbase.writer.unified.encoded.blocksize.ratio" isn't set, wes still consider the unencoded size for calculating block limit, which is the same with previous if condition on checkBlockBoundary method. The difference is when "hbase.writer.unified.encoded.blocksize.ratio" is set, as we now have 64KB of encoded data (whilst it would never be possible before).
--
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] bbeaudreault commented on a diff in pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4640:
URL: https://github.com/apache/hbase/pull/4640#discussion_r926652309
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterImpl.java:
##########
@@ -309,12 +311,15 @@ protected void finishInit(final Configuration conf) {
* At a block boundary, write all the inline blocks and opens new block.
*/
protected void checkBlockBoundary() throws IOException {
- // For encoder like prefixTree, encoded size is not available, so we have to compare both
- // encoded size and unencoded size to blocksize limit.
- if (
- blockWriter.encodedBlockSizeWritten() >= encodedBlockSizeLimit
- || blockWriter.blockSizeWritten() >= hFileContext.getBlocksize()
- ) {
+ boolean shouldFinishBlock = false;
+ //This means hbase.writer.unified.encoded.blocksize.ratio was set to something different from 0
Review Comment:
any implication on the comment that was here before which says we need to compare both? granted I think prefixTree has been removed
--
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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192862537
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | 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 _ |
| +1 :green_heart: | mvninstall | 2m 32s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 3m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 36s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 43s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 203m 36s | hbase-server in the patch passed. |
| | | 221m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 90c32740e9c7 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/testReport/ |
| Max. process+thread count | 2597 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/4/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 #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1192987660
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 2s | master passed |
| +1 :green_heart: | compile | 0m 35s | master passed |
| +1 :green_heart: | shadedjars | 3m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 9s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 198m 45s | hbase-server in the patch passed. |
| | | 214m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4640 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 37da3eeed84d 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 31fc97edee |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/testReport/ |
| Max. process+thread count | 2432 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4640/5/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] apurtell commented on pull request #4640: HBASE-27232 Fix checking for encoded block size when deciding if bloc…
Posted by GitBox <gi...@apache.org>.
apurtell commented on PR #4640:
URL: https://github.com/apache/hbase/pull/4640#issuecomment-1195961440
Master only? Not branch-2?
--
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