taklwu commented on code in PR #8384:
URL: https://github.com/apache/hbase/pull/8384#discussion_r3455563723
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAvoidCellReferencesIntoShippedBlocks.java:
##########
@@ -407,7 +415,8 @@ public void run() {
while (true) {
newBlockRefCount = 0;
newCacheList.clear();
- iterator = cache.iterator();
+ iterator = (Iterator<CachedBlock>)
CacheAccessServices.asCachedBlockIterable(cache)
+ .orElseThrow();
Review Comment:
ditto
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAvoidCellReferencesIntoShippedBlocks.java:
##########
@@ -230,7 +233,9 @@ public void run() {
}
}
List<BlockCacheKey> cacheList = new ArrayList<>();
- Iterator<CachedBlock> iterator = cache.iterator();
+ @SuppressWarnings("unchecked")
+ Iterator<CachedBlock> iterator =
+ (Iterator<CachedBlock>)
CacheAccessServices.asCachedBlockIterable(cache).orElseThrow();
Review Comment:
should this be the right change?
```suggestion
(Iterator<CachedBlock>)
CacheAccessServices.asCachedBlockIterable(cache).orElseThrow().iterator();;
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/CacheConfig.java:
##########
@@ -190,20 +191,56 @@ public class CacheConfig implements
PropagatingConfigurationObserver {
* @param conf hbase configuration
*/
public CacheConfig(Configuration conf) {
- this(conf, null);
+ this(conf, (CacheAccessService) null);
}
+ /**
+ * Create a cache configuration using the specified configuration object and
block cache. Only use
+ * in tests
+ * @param conf hbase configuration
+ * @param blockCache block cache to use for this configuration
+ */
public CacheConfig(Configuration conf, BlockCache blockCache) {
Review Comment:
with the new `public CacheConfig(Configuration conf, CacheAccessService
service) {` , will we migrate or remove this constructor in the future ?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAvoidCellReferencesIntoShippedBlocks.java:
##########
@@ -364,10 +369,12 @@ public void testHBASE16372InReadPath() throws Exception {
try (ScanPerNextResultScanner scanner =
new
ScanPerNextResultScanner(TEST_UTIL.getAsyncConnection().getTable(tableName),
s)) {
Thread evictorThread = new Thread() {
+ @SuppressWarnings("unchecked")
@Override
public void run() {
List<BlockCacheKey> cacheList = new ArrayList<>();
- Iterator<CachedBlock> iterator = cache.iterator();
+ Iterator<CachedBlock> iterator = (Iterator<CachedBlock>)
CacheAccessServices
+ .asCachedBlockIterable(cache).orElseThrow();
Review Comment:
ditto with `.iterator();` ?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java:
##########
@@ -490,7 +500,7 @@ private void
testCachingDataBlocksDuringCompactionInternals(boolean useTags,
// of testing
// BucketCache, we cannot verify block type as it is not stored in the
cache.
boolean cacheOnCompactAndNonBucketCache =
- cacheBlocksOnCompaction && !(blockCache instanceof BucketCache);
+ cacheBlocksOnCompaction && !(cache instanceof BucketCache);
Review Comment:
this is also right, please fix it.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java:
##########
@@ -148,70 +152,73 @@ public void modifyConf(Configuration conf) {
}
public TestCacheOnWrite(CacheOnWriteType cowType, Compression.Algorithm
compress,
- boolean cacheCompressedData, BlockCache blockCache) {
+ boolean cacheCompressedData, CacheAccessService blockCache) {
this.cowType = cowType;
this.compress = compress;
this.cacheCompressedData = cacheCompressedData;
- this.blockCache = blockCache;
+ this.cache = blockCache;
testDescription = "[cacheOnWrite=" + cowType + ", compress=" + compress
+ ", cacheCompressedData=" + cacheCompressedData + "]";
LOG.info(testDescription);
}
- private static List<BlockCache> getBlockCaches() throws IOException {
+ private static List<CacheAccessService> getCacheServices() throws
IOException {
Configuration conf = TEST_UTIL.getConfiguration();
- List<BlockCache> blockcaches = new ArrayList<>();
+ List<CacheAccessService> caches = new ArrayList<>();
// default
- blockcaches.add(BlockCacheFactory.createBlockCache(conf));
+ caches.add(CacheAccessServiceTestFactory.fromConfiguration(conf));
// set LruBlockCache.LRU_HARD_CAPACITY_LIMIT_FACTOR_CONFIG_NAME to 2.0f
due to HBASE-16287
TEST_UTIL.getConfiguration().setFloat(LruBlockCache.LRU_HARD_CAPACITY_LIMIT_FACTOR_CONFIG_NAME,
2.0f);
// memory
- BlockCache lru = new LruBlockCache(128 * 1024 * 1024, 64 * 1024,
TEST_UTIL.getConfiguration());
- blockcaches.add(lru);
+ CacheAccessService lru =
+ CacheAccessServiceTestFactory.lru(128 * 1024 * 1024, 64 * 1024,
TEST_UTIL.getConfiguration());
+ caches.add(lru);
// bucket cache
FileSystem.get(conf).mkdirs(TEST_UTIL.getDataTestDir());
int[] bucketSizes =
{ INDEX_BLOCK_SIZE, DATA_BLOCK_SIZE, BLOOM_BLOCK_SIZE, 64 * 1024, 128 *
1024 };
- BlockCache bucketcache =
- new BucketCache("offheap", 128 * 1024 * 1024, 64 * 1024, bucketSizes, 5,
64 * 100, null);
- blockcaches.add(bucketcache);
- return blockcaches;
+ CacheAccessService bucketcache =
CacheAccessServiceTestFactory.bucket("offheap",
+ 128 * 1024 * 1024, 64 * 1024, bucketSizes, 5, 64 * 100, null);
+ caches.add(bucketcache);
+ return caches;
}
public static Stream<Arguments> parameters() throws IOException {
List<Arguments> params = new ArrayList<>();
- for (BlockCache blockCache : getBlockCaches()) {
+ for (CacheAccessService cache : getCacheServices()) {
for (CacheOnWriteType cowType : CacheOnWriteType.values()) {
for (Compression.Algorithm compress :
HBaseCommonTestingUtil.COMPRESSION_ALGORITHMS) {
for (boolean cacheCompressedData : new boolean[] { false, true }) {
- params.add(Arguments.of(cowType, compress, cacheCompressedData,
blockCache));
+ params.add(Arguments.of(cowType, compress, cacheCompressedData,
cache));
}
}
}
}
return params.stream();
}
- private void clearBlockCache(BlockCache blockCache) throws
InterruptedException {
- if (blockCache instanceof LruBlockCache) {
- ((LruBlockCache) blockCache).clearCache();
+ private void clearBlockCache(CacheAccessService cache) throws
InterruptedException {
+ // TODO: HBASE-30018 refactor later
+ BlockCache blockCache = ((BlockCacheBackedCacheAccessService)
cache).getBlockCache();
+ if (cache instanceof LruBlockCache) {
+ ((LruBlockCache) cache).clearCache();
} else {
Review Comment:
This has the right comment, please fix it
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAvoidCellReferencesIntoShippedBlocks.java:
##########
@@ -383,7 +390,8 @@ public void run() {
Thread.sleep(1);
} catch (InterruptedException e1) {
}
- iterator = cache.iterator();
+ iterator = (Iterator<CachedBlock>)
CacheAccessServices.asCachedBlockIterable(cache)
+ .orElseThrow();
Review Comment:
ditto
--
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]
