wchevreuil commented on code in PR #6182:
URL: https://github.com/apache/hbase/pull/6182#discussion_r1741730401
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/CacheTestUtils.java:
##########
@@ -351,4 +351,8 @@ public static void getBlockAndAssertEquals(BlockCache
cache, BlockCacheKey key,
}
}
}
+
+ public static boolean waitForCacheInitialization(BlockCache cache, long
timeout) {
+ return cache.waitForCacheInitialization(timeout);
+ }
Review Comment:
You are just saving one line? Then better call
`cache.waitForCacheInitialization` directly on your individual tests, right?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCache.java:
##########
@@ -261,4 +261,22 @@ default Optional<Map<String, Long>> getRegionCachedInfo() {
default int evictBlocksRangeByHfileName(String hfileName, long initOffset,
long endOffset) {
return 0;
}
+
+ /**
+ * API to check whether or not, the cache is enabled.
+ * @return returns true if the cache is enabled, false otherwise.
+ */
+ default boolean isCacheEnabled() {
+ return true;
+ }
+
+ /**
+ * Wait for the bucket cache to be enabled while server restart
+ * @param cache cache object
+ * @param timeout time to wait for the bucket cache to be enable
+ * @return boolean trye if the bucket cache is enabled, false otherwise
+ */
+ default boolean waitForCacheInitialization(long timeout) {
Review Comment:
Please fix javadoc to address pre-commit validation warning.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestPrefetch.java:
##########
@@ -108,11 +109,13 @@ public class TestPrefetch {
public OpenTelemetryRule otelRule = OpenTelemetryRule.create();
@Before
- public void setUp() throws IOException {
+ public void setUp() throws IOException, InterruptedException {
conf = TEST_UTIL.getConfiguration();
conf.setBoolean(CacheConfig.PREFETCH_BLOCKS_ON_OPEN_KEY, true);
fs = HFileSystem.get(conf);
blockCache = BlockCacheFactory.createBlockCache(conf);
+ // Add some sleep to enable the cache to be instantiated.
+ Thread.sleep(2000);
Review Comment:
Is this needed for all tests in TestPrefetch? I guess not, as this is only
relevant for persistent cache. Better move the tests with async cache
initialisation to a separate test class.
--
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]
