You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2025/01/07 16:04:03 UTC
[PR] Replaced tserv block cache mgr property with a more general one [accumulo]
dlmarion opened a new pull request, #5231:
URL: https://github.com/apache/accumulo/pull/5231
The block cache manager is not only used in the tserver, it's also used in the sserver and by a client that might be reading from an RFile.
Closes #4663
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #5231:
URL: https://github.com/apache/accumulo/pull/5231
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5231:
URL: https://github.com/apache/accumulo/pull/5231#discussion_r1906047156
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -294,6 +294,12 @@ public enum Property {
+ " user-implementations of pluggable Accumulo features, such as the balancer"
+ " or volume chooser.",
"2.0.0"),
+ GENERAL_CACHE_MANAGER_IMPL("general.block.cache.manager.class",
+ "org.apache.accumulo.core.file.blockfile.cache.lru.LruBlockCacheManager", PropertyType.STRING,
+ "Specifies the class name of the block cache factory implementation."
+ + " Alternative implementation is"
+ + " org.apache.accumulo.core.file.blockfile.cache.tinylfu.TinyLfuBlockCacheManager.",
Review Comment:
The default is `LruBlockCacheManager`, I just copied the code from `TSERV_CACHE_MANAGER_IMPL`. The default was changed to the TinyLfuBlockCacheManager in a later release IIRC.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5231:
URL: https://github.com/apache/accumulo/pull/5231#discussion_r1906019351
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -294,6 +294,12 @@ public enum Property {
+ " user-implementations of pluggable Accumulo features, such as the balancer"
+ " or volume chooser.",
"2.0.0"),
+ GENERAL_CACHE_MANAGER_IMPL("general.block.cache.manager.class",
+ "org.apache.accumulo.core.file.blockfile.cache.lru.LruBlockCacheManager", PropertyType.STRING,
+ "Specifies the class name of the block cache factory implementation."
+ + " Alternative implementation is"
+ + " org.apache.accumulo.core.file.blockfile.cache.tinylfu.TinyLfuBlockCacheManager.",
Review Comment:
Why change the default impl from LruBlockCacheManager to TinyLfuBlockCacheManager?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5231:
URL: https://github.com/apache/accumulo/pull/5231#issuecomment-2577913650
I removed the deprecated property when I merged this into `main`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5231:
URL: https://github.com/apache/accumulo/pull/5231#issuecomment-2576239766
> Would it make sense in a follow on change to make the cache size props be general? Like the `tserver.cache.data.size` and `sserver.cache.data.size` props.
I think it's ok to leave them separate that way if you have different settings in the same resource group, then you won't need different configs.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replaced tserv block cache mgr property with a more general one [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on PR #5231:
URL: https://github.com/apache/accumulo/pull/5231#issuecomment-2576304251
Discovered #5234 when testing this PR.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org