taklwu commented on code in PR #8653:
URL: https://github.com/apache/hbase/pull/8653#discussion_r4068292936
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/cache/CacheAccessServices.java:
##########
@@ -81,33 +77,59 @@ public static CacheAccessService fromBlockCache(BlockCache
blockCache) {
DefaultHBaseCachePlacementAdmissionPolicy.INSTANCE);
}
+ /**
+ * Creates a {@link CacheAccessService} from the block cache configuration.
+ * @param conf cache configuration
+ * @return configured cache access service, or a disabled service when block
caching is disabled
+ * @throws NullPointerException if {@code conf} is {@code null}
+ */
+ public static CacheAccessService fromConfiguration(Configuration conf) {
+ return fromConfiguration(conf, null);
+ }
+
/**
* Creates a {@link CacheAccessService} from the block cache configuration.
* <p>
- * This method is a compatibility factory for tests and transitional code
paths that want to
- * obtain a {@link CacheAccessService} directly from {@link Configuration},
while still using the
- * existing {@link BlockCacheFactory} and legacy {@link BlockCache}
implementations underneath.
- * </p>
- * <p>
- * The method delegates block-cache construction to
- * {@link BlockCacheFactory#createBlockCache(Configuration)}. If the legacy
factory creates a
- * {@link BlockCache}, the returned service is backed by that cache through
- * {@link TopologyBackedCacheAccessService}. If the legacy factory does not
create a cache, this
- * method returns the disabled/no-op cache access service.
- * </p>
- * <p>
- * This method does not introduce new cache-engine or topology-based runtime
wiring. It is
- * intended only as a bridge while existing HBase tests and integration
paths migrate from direct
- * {@link BlockCache} usage to {@link CacheAccessService}.
+ * Cache implementations that implement {@link CacheEngine} natively are
used directly. Legacy
+ * {@link BlockCache} implementations are adapted to {@link CacheEngine}
until their migration is
+ * complete.
* </p>
- * @param conf configuration used by {@link BlockCacheFactory}
- * @return cache access service created from the configured legacy block
cache, or disabled when
- * no block cache is configured
+ * @param conf cache configuration
+ * @param onlineRegions currently online regions, or {@code null} when
unavailable
+ * @return configured cache access service, or a disabled service when block
caching is disabled
* @throws NullPointerException if {@code conf} is {@code null}
*/
- public static CacheAccessService fromConfiguration(Configuration conf) {
+ public static CacheAccessService fromConfiguration(Configuration conf,
+ Map<String, HRegion> onlineRegions) {
Objects.requireNonNull(conf, "conf must not be null");
- return fromBlockCache(BlockCacheFactory.createBlockCache(conf));
+
+ CacheEngine l1 = BlockCacheFactory.createFirstLevelCacheEngine(conf);
+ if (l1 == null) {
+ return disabled();
+ }
+
+ CachePlacementAdmissionPolicy policy =
DefaultHBaseCachePlacementAdmissionPolicy.INSTANCE;
+
+ boolean useExternal =
conf.getBoolean(BlockCacheFactory.EXTERNAL_BLOCKCACHE_KEY,
+ BlockCacheFactory.EXTERNAL_BLOCKCACHE_DEFAULT);
+
+ if (useExternal) {
+ CacheEngine l2 = BlockCacheFactory.createExternalCacheEngine(conf);
+ if (l2 == null) {
+ return
TopologyBackedCacheAccessServices.fromSingleCacheEngine("single", l1, policy);
+ }
+
+ return
TopologyBackedCacheAccessServices.fromTieredInclusiveCacheEngines("inclusive",
l1, l2,
+ policy);
+ }
+
+ CacheEngine l2 = BlockCacheFactory.createBucketCacheEngine(conf,
onlineRegions);
+ if (l2 == null) {
+ return TopologyBackedCacheAccessServices.fromSingleCacheEngine("single",
l1, policy);
+ }
+
+ return
TopologyBackedCacheAccessServices.fromTieredExclusiveCacheEngines("combined",
l1, l2,
+ policy);
Review Comment:
as long as we have test to guard around it, copilot review may prove to be
wrong.
can we port TestCombinedBlockCache.testCombinedBlockCacheStats to this use
case and cover 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
