You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "Ryan4253 (via GitHub)" <gi...@apache.org> on 2025/06/13 19:51:16 UTC
[PR] Enable blob_cache [kvrocks]
Ryan4253 opened a new pull request, #3027:
URL: https://github.com/apache/kvrocks/pull/3027
Closes #3026
## Descrption of Changes:
Make the shared block cache cache blobs too.
## Test Plan:
### Unit Test
```
[----------] Global test environment tear-down
[==========] 431 tests from 69 test suites ran. (71550 ms total)
[ PASSED ] 430 tests.
[ SKIPPED ] 1 test, listed below:
[ SKIPPED ] UseBitmap/RedisBitmapTest.BitfieldStringGetSetTest/0
```
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#discussion_r2152205147
##########
kvrocks.conf:
##########
@@ -320,6 +320,10 @@ max-replication-mb 0
# Default: 0
max-io-mb 0
+# Whether to cache blob files within the block cache.
+# Default: yes
+enable-blob-cache yes
Review Comment:
I'm good with disabling by default.
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2972260623
The idea general LGTM, but I also wonder, would the performance more depending on block_cache's `high_pri_pool_ratio` and `low_pri_pool_ratio` in `LRUCacheOptions`, since the large blob might easily effect the global block cache?
For Blobs, previous mode would cache the items and fetching the blobs, if the memory is enough for these blobs, not caching the blobs is a bad idea. But when memory is not enough, this mode might utililize SSD's random read
Current mode making blob items into cache, when memory is enough this just enhance the performance, but in other scenerio this would making depending on cache policies
Generally I think we can add a server side config for this( maybe default enable?) , and allowing config this when users see regression.
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "Ryan4253 (via GitHub)" <gi...@apache.org>.
Ryan4253 commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2976950024
Option added 👍
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): allow to enable the blob cache [kvrocks]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2980968946
## [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=3027) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=3027&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=3027&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=3027&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [59.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=3027&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=3027&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=3027)
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2972145365
Looks good to me, thank you. @mapleFU @PragmaTwice @torwig @aleksraiden Do you think we should add an option to opt out of this?
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): allow to enable the blob cache [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): allow to enable the blob cache [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2980396028
@Ryan4253 Thanks for your contribution.
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2976682946
> @git-hulk @mapleFU I agree that introducing this _can_ cause regression given limited memory capacity since blob files are less valuable caching targets. I can go ahead and add a config to disable caching blob files if you guys think thats would be more backward compatible. Thanks!
Yes, it would be nice to have an option to opt out if users find any performance regression.
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "Ryan4253 (via GitHub)" <gi...@apache.org>.
Ryan4253 commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2976648839
@git-hulk @mapleFU I agree that introducing this *can* cause regression given limited memory capacity since blob files are less valuable caching targets. I can go ahead and add a config to disable caching blob files if you guys think thats better. Thanks!
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): allow to enable the blob cache [kvrocks]
Posted by "Ryan4253 (via GitHub)" <gi...@apache.org>.
Ryan4253 commented on PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#issuecomment-2980348489
@git-hulk @torwig changed default to not enable blob cache
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "Ryan4253 (via GitHub)" <gi...@apache.org>.
Ryan4253 commented on code in PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#discussion_r2150987666
##########
kvrocks.conf:
##########
@@ -320,6 +320,10 @@ max-replication-mb 0
# Default: 0
max-io-mb 0
+# Whether to cache blob files within the block cache.
+# Default: yes
+enable-blob-cache yes
Review Comment:
Either works for me, just let me know which 👍
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(storage): Enable blob cache [kvrocks]
Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on code in PR #3027:
URL: https://github.com/apache/kvrocks/pull/3027#discussion_r2150506185
##########
kvrocks.conf:
##########
@@ -320,6 +320,10 @@ max-replication-mb 0
# Default: 0
max-io-mb 0
+# Whether to cache blob files within the block cache.
+# Default: yes
+enable-blob-cache yes
Review Comment:
@git-hulk Perhaps we can disable this feature by default, and if users want to activate it and reap the benefits, they can turn it on. WDYT?
--
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@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org