You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "Amemeda (via GitHub)" <gi...@apache.org> on 2026/10/16 18:50:54 UTC
[PR] Remove deprecated LRUCacheBlock [accumulo]
Amemeda opened a new pull request, #6596:
URL: https://github.com/apache/accumulo/pull/6596
(no comment)
--
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] Remove deprecated LRUCacheBlock [accumulo]
Posted by "Amemeda (via GitHub)" <gi...@apache.org>.
Amemeda commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6222917444
There are 4 files remaining in the cache.lru subfolder where all the lru files were deleted from. The only usage of these files is in `TestCachedBlockQueue.java` which is testing using lru.CachedBlock.java. All tests in this file still pass with the LRU deleted. So I am wondering, should `TestCachedBlockQueue.java` be deleted along with the remaining 4 files in lru package? Or I can refactor the remaining 4 files from the lru package, probably to rename the package to util or something similar.
<img width="430" height="290" alt="image" src="https://github.com/user-attachments/assets/12544005-6bad-445c-bd4b-58fd34e25021" />
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "dlmarion (via GitHub) via notifications" <no...@accumulo.apache.org>.
dlmarion commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6740835658
@Amemeda - can you merge `main` into this branch to get it up to date. Then I'll merge 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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove deprecated LRUBlockCache [accumulo]
Posted by "Amemeda (via GitHub)" <gi...@apache.org>.
Amemeda commented on code in PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#discussion_r4314582524
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/tinylfu/SynchronousLoadingBlockCache.java:
##########
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.core.file.blockfile.cache.lru;
+package org.apache.accumulo.core.file.blockfile.cache.tinylfu;
Review Comment:
updated in c433118fc6fc1dc3b19f0ebe15a22b63c672c7b3
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "DomGarguilo (via GitHub) via notifications" <no...@accumulo.apache.org>.
DomGarguilo commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6772457752
> There is a method in BlockCacheManager.java that still has documentation referencing LRU. Unsure if this needs to be updated to TinyLFU or its talking about a different LRU like the LRU Map I see throughout the code
>
> /**
> * This method provides a way for a cache implementation to access arbitrary configuration set
> * by a user.
> *
> * <p>
> * When running in a tserver, returns all Accumulo properties that have a prefix of
> * {@code tserver.cache.config.<prefix>.<type>.} or
> * {@code tserver.cache.config.<prefix>.default.} with values for specific cache types
> * overriding defaults.
> *
> * <p>
> * When running in a scan server, returns all Accumulo properties that have a prefix of
> * {@code sserver.cache.config.<prefix>.<type>.} or
> * {@code sserver.cache.config.<prefix>.default.} with values for specific cache types
> * overriding defaults.
> *
> * <p>
> * For example assume the following data is in Accumulo's system config and the plugin is
> * running in a tserver.
> *
> * <pre>
> * tserver.cache.config.lru.default.evictAfter=3600
> * tserver.cache.config.lru.default.loadFactor=.75
> * tserver.cache.config.lru.index.loadFactor=.55
> * tserver.cache.config.lru.data.loadFactor=.65
> * </pre>
> *
> * <p>
> * If this method is called with {@code prefix=lru} and {@code type=INDEX} then it would return
> * a map with the following key values. The load factor setting for index overrides the default
> * value.
> *
> * <pre>
> * evictAfter=3600
> * loadFactor=.55
> * </pre>
> *
> * @param prefix A unique identifier that corresponds to a particular BlockCacheManager
> * implementation.
> */
> Map<String,String> getProperties(String prefix, CacheType type);
Yea I think this should be updated. Maybe just swap the example to use a generic prefix like `tserver.cache.config.myplugin.default.*` with `prefix=myplugin` for example.
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "Amemeda (via GitHub)" <gi...@apache.org>.
Amemeda commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6224632228
There is a method in BlockCacheManager.java that still has documentation referencing LRU. Unsure if this needs to be updated to TinyLFU or its talking about a different LRU like the LRU Map I see throughout the code
```java
/**
* This method provides a way for a cache implementation to access arbitrary configuration set
* by a user.
*
* <p>
* When running in a tserver, returns all Accumulo properties that have a prefix of
* {@code tserver.cache.config.<prefix>.<type>.} or
* {@code tserver.cache.config.<prefix>.default.} with values for specific cache types
* overriding defaults.
*
* <p>
* When running in a scan server, returns all Accumulo properties that have a prefix of
* {@code sserver.cache.config.<prefix>.<type>.} or
* {@code sserver.cache.config.<prefix>.default.} with values for specific cache types
* overriding defaults.
*
* <p>
* For example assume the following data is in Accumulo's system config and the plugin is
* running in a tserver.
*
* <pre>
* tserver.cache.config.lru.default.evictAfter=3600
* tserver.cache.config.lru.default.loadFactor=.75
* tserver.cache.config.lru.index.loadFactor=.55
* tserver.cache.config.lru.data.loadFactor=.65
* </pre>
*
* <p>
* If this method is called with {@code prefix=lru} and {@code type=INDEX} then it would return
* a map with the following key values. The load factor setting for index overrides the default
* value.
*
* <pre>
* evictAfter=3600
* loadFactor=.55
* </pre>
*
* @param prefix A unique identifier that corresponds to a particular BlockCacheManager
* implementation.
*/
Map<String,String> getProperties(String prefix, CacheType type);
```
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#discussion_r4313241306
##########
hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/lib/ConfiguratorBase.java:
##########
@@ -161,7 +161,7 @@ public static AccumuloClient createClient(Class<?> implementingClass, Configurat
* Sets the valid visibility count for this job.
*
* @param conf the Hadoop configuration object to configure
- * @param visibilityCacheSize the LRU cache size
+ * @param visibilityCacheSize the Tiny LFU cache size
Review Comment:
```suggestion
* @param visibilityCacheSize the cache size
```
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/tinylfu/SynchronousLoadingBlockCache.java:
##########
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.accumulo.core.file.blockfile.cache.lru;
+package org.apache.accumulo.core.file.blockfile.cache.tinylfu;
Review Comment:
I would move this into the spi/cache package. Only the LRUBlockCache extended this, but it could serve as a good base class for other implementations.
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "DomGarguilo (via GitHub) via notifications" <no...@accumulo.apache.org>.
DomGarguilo commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6772420089
> SynchronizedLoadingBlockCache.java: Moved to spi.cache subfolder. Still used elsewhere in code.
I can't find where else its used in the code? I don't think moving it into `spi.cache` is a good idea since that makes it public API. Could we just delete it instead?
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "Amemeda (via GitHub) via notifications" <no...@accumulo.apache.org>.
Amemeda commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6828155234
> I can't find where else its used in the code? I don't think moving it into spi.cache is a good idea since that makes it public API. Could we just delete it instead?
SynchronizedLoadingBlochCache has no usages, but it is referenced in BlockCache.getBlock documentation, so I was assuming it was still usable, if not I will remove it. One of its
```java
/**
* This method allows a cache to prevent concurrent loads of the same block. However a cache
* implementation is not required to prevent concurrent loads.
* {@link SynchronousLoadingBlockCache} is an abstract class that a cache can extent which does
* prevent concurrent loading of the same block.
*
*
* @param blockName Block name to fetch
* @param loader If the block is not present in the cache, the loader can be called to load it.
* @return Block or null if block is not in the cache or didn't load.
*/
CacheEntry getBlock(String blockName, Loader loader);
```
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "Amemeda (via GitHub)" <gi...@apache.org>.
Amemeda commented on code in PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#discussion_r4314580247
##########
hadoop-mapreduce/src/main/java/org/apache/accumulo/hadoopImpl/mapreduce/lib/ConfiguratorBase.java:
##########
@@ -161,7 +161,7 @@ public static AccumuloClient createClient(Class<?> implementingClass, Configurat
* Sets the valid visibility count for this job.
*
* @param conf the Hadoop configuration object to configure
- * @param visibilityCacheSize the LRU cache size
+ * @param visibilityCacheSize the Tiny LFU cache size
Review Comment:
updated in c433118fc6fc1dc3b19f0ebe15a22b63c672c7b3
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6223114313
I think you can remove TestCachedBlockQueue as well
--
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] Remove deprecated LRUBlockCache [accumulo]
Posted by "DomGarguilo (via GitHub) via notifications" <no...@accumulo.apache.org>.
DomGarguilo commented on PR #6596:
URL: https://github.com/apache/accumulo/pull/6596#issuecomment-6772542044
Users who have `general.block.cache.manager.class` set to LruBlockCacheManager will fail to start after upgrading to 4.0. Could we add a release note telling them to unset the property or switch 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