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/02/24 14:16:51 UTC
[GitHub] [hbase] pustota2009 opened a new pull request #1200: Hbase 23887
BlockCache performance improve
pustota2009 opened a new pull request #1200: Hbase 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200
Caching only part of DATA blocks and LruBlockCache starts to work and save huge amount of GC
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But anyway probability to hit into cache definite by size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the feature - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (198 in the case above).
2. Populate the cache by data blocks anyway when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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 #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-590854368
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | yamllint | 0m 0s | yamllint was not available. |
| +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 1 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 52s | master passed |
| +1 :green_heart: | compile | 3m 21s | master passed |
| +1 :green_heart: | checkstyle | 2m 31s | master passed |
| +1 :green_heart: | shadedjars | 5m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 35s | master passed |
| +0 :ok: | spotbugs | 4m 11s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 19m 41s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 35s | the patch passed |
| +1 :green_heart: | compile | 3m 22s | the patch passed |
| +1 :green_heart: | javac | 3m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 29s | root: The patch generated 0 new + 10 unchanged - 1 fixed = 10 total (was 11) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | shadedjars | 0m 11s | patch has 7 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 43s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 3m 34s | the patch passed |
| +1 :green_heart: | findbugs | 20m 4s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 76m 15s | root in the patch failed. |
| -1 :x: | asflicense | 0m 49s | The patch generated 1 ASF License warnings. |
| | | 179m 35s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.regionserver.wal.TestFSHLog |
| | hadoop.hbase.TestJMXConnectorServer |
| | hadoop.hbase.regionserver.wal.TestAsyncWALReplayCompressed |
| | hadoop.hbase.regionserver.TestHRegionServerBulkLoad |
| | hadoop.hbase.regionserver.wal.TestSecureAsyncWALReplay |
| | hadoop.hbase.regionserver.TestHeapMemoryManager |
| | hadoop.hbase.regionserver.wal.TestWALReplayBoundedLogWriterCreation |
| | hadoop.hbase.TestMultiVersions |
| | hadoop.hbase.regionserver.wal.TestAsyncWALReplay |
| | hadoop.hbase.regionserver.wal.TestWALReplayCompressed |
| | hadoop.hbase.regionserver.wal.TestAsyncLogRollPeriod |
| | hadoop.hbase.regionserver.wal.TestAsyncLogRolling |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1200 |
| Optional Tests | dupname asflicense yamllint javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 222a3d1edc00 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-1200/out/precommit/personality/provided.sh |
| git revision | master / 3e21dc73de |
| Default Java | 1.8.0_181 |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/3/artifact/out/patch-shadedjars.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/3/artifact/out/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/3/testReport/ |
| asflicense | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/3/artifact/out/patch-asflicense-problems.txt |
| Max. process+thread count | 6439 (vs. ulimit of 10000) |
| modules | C: hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/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
[GitHub] [hbase] Apache-HBase commented on issue #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-590572687
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | yamllint | 0m 0s | yamllint was not available. |
| +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 1 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 47s | master passed |
| +1 :green_heart: | compile | 3m 21s | master passed |
| +1 :green_heart: | checkstyle | 2m 31s | master passed |
| +1 :green_heart: | shadedjars | 5m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 31s | master passed |
| +0 :ok: | spotbugs | 4m 13s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 19m 48s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 34s | the patch passed |
| +1 :green_heart: | compile | 3m 25s | the patch passed |
| +1 :green_heart: | javac | 3m 25s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 29s | root: The patch generated 0 new + 8 unchanged - 3 fixed = 8 total (was 11) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | shadedjars | 0m 11s | patch has 7 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 18m 8s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 3m 37s | the patch passed |
| +1 :green_heart: | findbugs | 20m 50s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 132m 11s | root in the patch failed. |
| -1 :x: | asflicense | 0m 57s | The patch generated 1 ASF License warnings. |
| | | 237m 3s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.snapshot.TestExportSnapshotV1NoCluster |
| | hadoop.hbase.snapshot.TestExportSnapshotV2NoCluster |
| | hadoop.hbase.rsgroup.TestRSGroupsWithACL |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1200 |
| Optional Tests | dupname asflicense yamllint javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 3d4883810dc2 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-1200/out/precommit/personality/provided.sh |
| git revision | master / 69d683afcb |
| Default Java | 1.8.0_181 |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/2/artifact/out/patch-shadedjars.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/2/artifact/out/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/2/testReport/ |
| asflicense | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/2/artifact/out/patch-asflicense-problems.txt |
| Max. process+thread count | 6643 (vs. ulimit of 10000) |
| modules | C: hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/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] Apache-HBase commented on issue #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-590447723
: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. |
| +0 :ok: | yamllint | 0m 0s | yamllint was not available. |
| +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 1 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 36s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 18s | master passed |
| +1 :green_heart: | compile | 3m 7s | master passed |
| +1 :green_heart: | checkstyle | 2m 14s | master passed |
| +1 :green_heart: | shadedjars | 4m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 26s | master passed |
| +0 :ok: | spotbugs | 3m 56s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 18m 5s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 10s | the patch passed |
| +1 :green_heart: | compile | 3m 3s | the patch passed |
| +1 :green_heart: | javac | 3m 3s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 7s | root: The patch generated 0 new + 8 unchanged - 3 fixed = 8 total (was 11) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | shadedjars | 0m 12s | patch has 7 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 5s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 3m 32s | the patch passed |
| +1 :green_heart: | findbugs | 19m 50s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 83m 0s | root in the patch failed. |
| -1 :x: | asflicense | 1m 6s | The patch generated 1 ASF License warnings. |
| | | 180m 33s | |
| Reason | Tests |
|-------:|:------|
| Unreaped Processes | root:2 |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1200 |
| Optional Tests | dupname asflicense yamllint javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 4f5b6270b382 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-1200/out/precommit/personality/provided.sh |
| git revision | master / 418234356f |
| Default Java | 1.8.0_181 |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/artifact/out/patch-shadedjars.txt |
| Unreaped Processes Log | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/artifact/out/patch-unit-root-reaper.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/artifact/out/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/testReport/ |
| asflicense | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/1/artifact/out/patch-asflicense-problems.txt |
| Max. process+thread count | 8666 (vs. ulimit of 10000) |
| modules | C: hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1200/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] pustota2009 closed pull request #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 closed pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200
----------------------------------------------------------------
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] saintstack commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383668335
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -299,15 +307,15 @@ public LruBlockCache(long maxSize, long blockSize, Configuration conf) {
* @param memoryFactor percentage of total size for in-memory blocks
*/
public LruBlockCache(long maxSize, long blockSize, boolean evictionThread,
- int mapInitialSize, float mapLoadFactor, int mapConcurrencyLevel,
- float minFactor, float acceptableFactor, float singleFactor,
- float multiFactor, float memoryFactor, float hardLimitFactor,
- boolean forceInMemory, long maxBlockSize) {
+ int mapInitialSize, float mapLoadFactor, int mapConcurrencyLevel,
+ float minFactor, float acceptableFactor, float singleFactor,
+ float multiFactor, float memoryFactor, float hardLimitFactor,
+ boolean forceInMemory, long maxBlockSize, int cacheDataBlockPercent) {
Review comment:
Best to leave these sort of refactorings out of your patch. They bulk it up and distract from what is important about your change. FYI for next time.
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But anyway probability to hit into cache definite by size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (198 in the case above).
2. Populate the cache data blocks any way when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But any way probability to hit into cache definite size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (118 in the case above).
2. Populate the cache data blocks any way when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383682171
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -299,15 +307,15 @@ public LruBlockCache(long maxSize, long blockSize, Configuration conf) {
* @param memoryFactor percentage of total size for in-memory blocks
*/
public LruBlockCache(long maxSize, long blockSize, boolean evictionThread,
- int mapInitialSize, float mapLoadFactor, int mapConcurrencyLevel,
- float minFactor, float acceptableFactor, float singleFactor,
- float multiFactor, float memoryFactor, float hardLimitFactor,
- boolean forceInMemory, long maxBlockSize) {
+ int mapInitialSize, float mapLoadFactor, int mapConcurrencyLevel,
+ float minFactor, float acceptableFactor, float singleFactor,
+ float multiFactor, float memoryFactor, float hardLimitFactor,
+ boolean forceInMemory, long maxBlockSize, int cacheDataBlockPercent) {
Review comment:
Thanks! Will correct soon)
----------------------------------------------------------------
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] pustota2009 commented on issue #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-592321666
Should I correct erors in asflicense, unit and shadedjars? Looks like the problems not in the new code. If I could fix please help, how to do it.
----------------------------------------------------------------
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] pustota2009 commented on issue #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-595302577
@saintstack
Can you explain, please, could I do something to help with merging this feature? Maybe explain something more or just wait?
----------------------------------------------------------------
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] saintstack commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383669830
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
What is offset here? Bytes into a block? What is this modulus on offset doing? We might cache only some of the data blocks?
I don't understand how this gets you the graphs you show in the JIRA. How do you explain it?
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But any way probability to hit into cache defenite size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (118 in the case above).
2. Populate the cache data blocks any way when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But anyway probability to hit into cache definite by size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (198 in the case above).
2. Populate the cache by data blocks anyway when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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] pustota2009 edited a comment on issue #1200: HBASE 23887
BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 edited a comment on issue #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#issuecomment-595302577
@saintstack
Could you explain, please, can I do something to help with merging this feature? Maybe explain something more or just wait?
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But any way probability to hit into cache definite by size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (118 in the case above).
2. Populate the cache data blocks any way when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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] pustota2009 commented on a change in pull request #1200:
HBASE 23887 BlockCache performance improve
Posted by GitBox <gi...@apache.org>.
pustota2009 commented on a change in pull request #1200: HBASE 23887 BlockCache performance improve
URL: https://github.com/apache/hbase/pull/1200#discussion_r383691217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
##########
@@ -400,16 +413,24 @@ private Cacheable asReferencedHeapBlock(Cacheable buf) {
*/
@Override
public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf, boolean inMemory) {
+ if (cacheDataBlockPercent != 100 && buf.getBlockType().isData()) {
+ // Don't cache this DATA block if we have limit on BlockCache,
+ // good for performance (HBASE-23887)
+ if (cacheKey.getOffset() % 100 >= cacheDataBlockPercent) {
Review comment:
Yes, I think you got the general idea - when data much more then the cache we have overhead. Lets provide some details. Why we do a lot of job:
1. Put some key-value into the map
2. Delete it quite soon, because there are quite big queue at entrance.
3. Clean the garbage
But anyway probability to hit into cache definite by size of cache. That is why we can put into cache some random blocks and skip 1-3 steps for the rest blocks. How it works:
Imagine we have little cache. Just for 1 block and trying to read 3 blocks with offsets (into files):
124
198
223
Original old way - we put the block 124, then put 198, evict 124, put 223, evict 198. A lot of work (5 actions).
With the patch - last few digits evenly distributed from 0 to 99. When we divide by modulus we got:
124 -> 24
198 -> 98
223 -> 23
It helps to sort them. Some part, for example below 50 (if we set cacheDataBlockPercent = 50) go into the cache. And skip others. It means we will not try work with the block 198 and save CPU for other job. In the result - we put block 124, then put 223, evict 124 (3 actions). Thats the idea.
If all is ok, later we can do some little improvments:
1. Don't even try get from the cache DATA blocks which can't be there (118 in the case above).
2. Populate the cache data blocks any way when the cache is empty.
And looks as good improvment provide the same logic for L2.
----------------------------------------------------------------
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