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 2020/01/20 09:22:48 UTC
[GitHub] [hbase] abhinabasarkar opened a new pull request #1074: Implemented
threshold for cache on compaction
abhinabasarkar opened a new pull request #1074: Implemented threshold for cache on compaction
URL: https://github.com/apache/hbase/pull/1074
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-579607860
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 0s | Docker mode activated. |
| -1 :x: | patch | 0m 4s | https://github.com/apache/hbase/pull/1074 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/4/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] abhinabasarkar closed pull request #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
abhinabasarkar closed pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368722943
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1137,7 +1146,13 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
cacheOnWriteLogged = true;
}
} else {
- writerCacheConf.setCacheDataOnWrite(false);
+ writerCacheConf.disableCacheOnWrite();
+ if (totalCompactedFileSize > cacheConf.getCacheCompactedBlocksOnWriteThreshold()) {
+ // checking condition once again for logging
+ LOG.debug("Setting data on write as false as total size of compacted files "
+ + totalCompactedFileSize + "is greater than cache compacted blocks on write threshold "
+ + cacheConf.getCacheCompactedBlocksOnWriteThreshold());
Review comment:
same here, use parameterized log with {}
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] abhinabasarkar commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
abhinabasarkar commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-579710966
How to fix the checkstyle issue for visibility modifier? This is new checkstyle issue that this patch is generating -
> ./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:134: public long totalCompactedFilesSize = 0;:17: Variable 'totalCompactedFilesSize' must be private and have accessor methods. [VisibilityModifier]
In the root checkstyle file
> https://builds.apache.org/job/HBase%20Nightly/job/branch-2/lastSuccessfulBuild/artifact/output-general/patch-checkstyle-root.txt
`Compactor.java` already has similar checkstyle warnings.
> ./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:120:17: Variable 'maxKeyCount' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:122:17: Variable 'earliestPutTs' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:124:17: Variable 'latestPutTs' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:126:17: Variable 'maxSeqId' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:128:17: Variable 'maxMVCCReadpoint' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:130:16: Variable 'maxTagsLength' must be private and have accessor methods. [VisibilityModifier]
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java:132:17: Variable 'minSeqIdToKeep' must be private and have accessor methods. [VisibilityModifier]
cc @HorizonNet @busbey
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HorizonNet commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r370675993
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
##########
@@ -508,18 +522,31 @@ private void testCachingDataBlocksDuringCompactionInternals(boolean useTags,
"\ncacheBlocksOnCompaction: "
+ cacheBlocksOnCompaction + "\n";
- assertEquals(assertErrorMessage, cacheOnCompactAndNonBucketCache, dataBlockCached);
+ if(cacheOnCompactAndNonBucketCache && cacheBlocksOnCompactionThreshold > 0) {
Review comment:
White space after `if`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HorizonNet commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r370675418
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/Compactor.java
##########
@@ -130,6 +130,8 @@ public CompactionProgress getProgress() {
public int maxTagsLength = 0;
/** Min SeqId to keep during a major compaction **/
public long minSeqIdToKeep = 0;
+ /** Total file size of the compacted files **/
Review comment:
'files' or 'file'? If first one, please change the field name. Otherwise, change the documentation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
virajjasani commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-579622879
@abhinabasarkar Please rebase your branch once
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-579707263
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 14s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 5 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 41s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 48s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | checkstyle | 2m 33s | master passed |
| +1 :green_heart: | shadedjars | 5m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | master passed |
| +0 :ok: | spotbugs | 4m 24s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 14s | branch/hbase-checkstyle no findbugs output file (findbugsXml.xml) |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 33s | the patch passed |
| +1 :green_heart: | compile | 1m 9s | the patch passed |
| +1 :green_heart: | javac | 1m 9s | the patch passed |
| -1 :x: | checkstyle | 2m 29s | root: The patch generated 5 new + 70 unchanged - 0 fixed = 75 total (was 70) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedjars | 5m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 18m 10s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed |
| +0 :ok: | findbugs | 0m 12s | hbase-checkstyle has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 11s | hbase-checkstyle in the patch passed. |
| +1 :green_heart: | unit | 161m 24s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 52s | The patch does not generate ASF License warnings. |
| | | 229m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Optional Tests | dupname asflicense checkstyle javac javadoc unit xml spotbugs findbugs shadedjars hadoopcheck hbaseanti compile |
| uname | Linux e106f8997486 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1074/out/precommit/personality/provided.sh |
| git revision | master / 98cff8a26a |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/5/artifact/out/diff-checkstyle-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/5/testReport/ |
| Max. process+thread count | 4805 (vs. ulimit of 10000) |
| modules | C: hbase-checkstyle hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/5/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
virajjasani commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-576428335
Test case failures seem relevant
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-580267234
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 5 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 50s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | checkstyle | 2m 31s | master passed |
| +1 :green_heart: | shadedjars | 5m 3s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | master passed |
| +0 :ok: | spotbugs | 4m 34s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +0 :ok: | findbugs | 0m 15s | branch/hbase-checkstyle no findbugs output file (findbugsXml.xml) |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +1 :green_heart: | shadedjars | 5m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 7s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed |
| +0 :ok: | findbugs | 0m 11s | hbase-checkstyle has no data from findbugs |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 11s | hbase-checkstyle in the patch passed. |
| -1 :x: | unit | 155m 24s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 50s | The patch does not generate ASF License warnings. |
| | | 222m 5s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.security.access.TestAccessController3 |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Optional Tests | dupname asflicense checkstyle javac javadoc unit xml spotbugs findbugs shadedjars hadoopcheck hbaseanti compile |
| uname | Linux 7d5f7f2ab430 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1074/out/precommit/personality/provided.sh |
| git revision | master / 8b00f9f0b1 |
| Default Java | 1.8.0_181 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/6/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/6/testReport/ |
| Max. process+thread count | 5208 (vs. ulimit of 10000) |
| modules | C: hbase-checkstyle hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/6/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368721998
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
##########
@@ -87,6 +87,15 @@
public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY =
"hbase.rs.cachecompactedblocksonwrite";
+ /**
+ * Configuration key to determine total size in bytes of compacted files beyond which we do not cache blocks on compaction
+ */
+ public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD_KEY =
+ "hbase.rs.cachecompactedblocksonwritethreshold";
+
+ /**
+ * Configuration key to determine threshold beyond which cache on compaction should not work
Review comment:
Agree, I believe this can be 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368722533
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
##########
@@ -412,6 +447,19 @@ public ByteBuffAllocator getByteBuffAllocator() {
return this.byteBuffAllocator;
}
+ private long getCacheCompactedBlocksOnWriteThreshold(Configuration conf) {
+ long cacheCompactedBlocksOnWriteThreshold = conf.getLong(CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD_KEY,
+ DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD);
+
+ if (cacheCompactedBlocksOnWriteThreshold < 0) {
+ LOG.warn("cacheCompactedBlocksOnWriteThreshold value :" + cacheCompactedBlocksOnWriteThreshold
+ + "is less than 0, resetting it to: " + DEFAULT_CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD);
Review comment:
We can use Log with {} for arguments similar to other usecases:
```
LOG.warn("cacheCompactedBlocksOnWriteThreshold value : {} is less than 0, resetting it to: ", cacheCompactedBlocksOnWriteThreshold)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HorizonNet commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r370673495
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
##########
@@ -288,6 +304,19 @@ public void enableCacheOnWrite() {
this.cacheBloomsOnWrite = true;
}
+ /**
+ * Disable cache on write including:
+ * cacheDataOnWrite
+ * cacheIndexesOnWrite
+ * cacheBloomsOnWrite
+ */
+ public void disableCacheOnWrite() {
+ this.cacheDataOnWrite = false;
+ this.cacheIndexesOnWrite = false;
+ this.cacheBloomsOnWrite = false;
+ }
+
Review comment:
Remove this additional empty line.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] anoopsjohn commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368654059
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
##########
@@ -87,6 +87,15 @@
public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY =
"hbase.rs.cachecompactedblocksonwrite";
+ /**
+ * Configuration key to determine total size in bytes of compacted files beyond which we do not cache blocks on compaction
+ */
+ public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD_KEY =
+ "hbase.rs.cachecompactedblocksonwritethreshold";
Review comment:
Name it like hbase.rs.cachecompactedblocksonwrite.threshold
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ramkrish86 commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
ramkrish86 commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-580364100
Will commit tihs unless objections.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-576271863
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 46s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | checkstyle | 1m 14s | master passed |
| +1 :green_heart: | shadedjars | 4m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
| +0 :ok: | spotbugs | 4m 54s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 52s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 25s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| -1 :x: | checkstyle | 1m 10s | hbase-server: The patch generated 5 new + 65 unchanged - 0 fixed = 70 total (was 65) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 21s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 19m 42s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
| +1 :green_heart: | findbugs | 5m 46s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 164m 21s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 231m 7s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.client.TestFromClientSideWithCoprocessor |
| | hadoop.hbase.client.TestFromClientSide |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 375afec3cd23 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1074/out/precommit/personality/provided.sh |
| git revision | master / 569ac1232a |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/1/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/1/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/1/testReport/ |
| Max. process+thread count | 5632 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-576623788
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 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 1s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 4m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
| +0 :ok: | spotbugs | 4m 30s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 59s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| -1 :x: | checkstyle | 1m 10s | hbase-server: The patch generated 5 new + 65 unchanged - 0 fixed = 70 total (was 65) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 37s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 38s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
| +1 :green_heart: | findbugs | 4m 41s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 164m 22s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 34s | The patch does not generate ASF License warnings. |
| | | 222m 7s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.io.hfile.TestCacheOnWrite |
| | hadoop.hbase.master.replication.TestRegisterPeerWorkerWhenRestarting |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux d9fb68d6b4d9 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1074/out/precommit/personality/provided.sh |
| git revision | master / 569ac1232a |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/2/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/2/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/2/testReport/ |
| Max. process+thread count | 5721 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] anoopsjohn commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368657370
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1118,17 +1125,19 @@ private HStoreFile commitFile(Path path, long logCacheFlushId, MonitoredTask sta
// compaction
public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm compression,
boolean isCompaction, boolean includeMVCCReadpoint, boolean includesTag,
- boolean shouldDropBehind) throws IOException {
+ boolean shouldDropBehind, long totalCompactedFileSize) throws IOException {
Review comment:
totalCompactedFile's'Size
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] anoopsjohn commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368654722
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java
##########
@@ -87,6 +87,15 @@
public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_KEY =
"hbase.rs.cachecompactedblocksonwrite";
+ /**
+ * Configuration key to determine total size in bytes of compacted files beyond which we do not cache blocks on compaction
+ */
+ public static final String CACHE_COMPACTED_BLOCKS_ON_WRITE_THRESHOLD_KEY =
+ "hbase.rs.cachecompactedblocksonwritethreshold";
+
+ /**
+ * Configuration key to determine threshold beyond which cache on compaction should not work
Review comment:
Is this new comment make sense for below existing conf name? Am I missing some thing?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] anoopsjohn commented on a change in pull request #1074:
HBASE-23350 Make compaction files cacheonWrite configurable based on
threshold
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#discussion_r368657940
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1137,7 +1146,13 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
cacheOnWriteLogged = true;
}
} else {
- writerCacheConf.setCacheDataOnWrite(false);
+ writerCacheConf.disableCacheOnWrite();
+ if (totalCompactedFileSize > cacheConf.getCacheCompactedBlocksOnWriteThreshold()) {
Review comment:
Good.
may be we can log this line if the conf says to cache it but size is more. I would say use INFO log then.
Also to create the msg use the log statement like above way
LOG.info("For Store {} , cacheCompactedBlocksOnWrite....
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1074: HBASE-23350 Make
compaction files cacheonWrite configurable based on threshold
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1074: HBASE-23350 Make compaction files cacheonWrite configurable based on threshold
URL: https://github.com/apache/hbase/pull/1074#issuecomment-576710042
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 0s | master passed |
| +1 :green_heart: | compile | 1m 14s | master passed |
| +1 :green_heart: | checkstyle | 1m 30s | master passed |
| +1 :green_heart: | shadedjars | 6m 19s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 46s | master passed |
| +0 :ok: | spotbugs | 5m 35s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 5m 32s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 39s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| -1 :x: | checkstyle | 1m 28s | hbase-server: The patch generated 5 new + 65 unchanged - 0 fixed = 70 total (was 65) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 20m 43s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
| +1 :green_heart: | findbugs | 5m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 160m 1s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 233m 20s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.client.TestFromClientSideWithCoprocessor |
| | hadoop.hbase.client.TestFromClientSide |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1074 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux ea77c4985263 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1074/out/precommit/personality/provided.sh |
| git revision | master / 569ac1232a |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/3/artifact/out/diff-checkstyle-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/3/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/3/testReport/ |
| Max. process+thread count | 5586 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1074/3/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services