wchevreuil commented on code in PR #6182:
URL: https://github.com/apache/hbase/pull/6182#discussion_r1735896937
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestPrefetch.java:
##########
@@ -364,16 +366,20 @@ public void testPrefetchWithDelay() throws Exception {
// Wait for 20 seconds, no thread should start prefetch
Thread.sleep(20000);
assertFalse("Prefetch threads should not be running at this point",
reader.prefetchStarted());
- while (!reader.prefetchStarted()) {
- assertTrue("Prefetch delay has not been expired yet",
- getElapsedTime(startTime) < PrefetchExecutor.getPrefetchDelay());
- }
- if (reader.prefetchStarted()) {
- // Added some delay as we have started the timer a bit late.
+ long timeout = 10000;
+ while (!reader.prefetchStarted() && !reader.prefetchComplete()) {
+ // Wait until the prefetch is triggered.
Thread.sleep(500);
- assertTrue("Prefetch should start post configured delay",
- getElapsedTime(startTime) > PrefetchExecutor.getPrefetchDelay());
+ if (timeout <= 0) break;
+ timeout -= 500;
}
Review Comment:
Use `Waiter.waitFor(Configuration conf, long timeout, long interval,
Predicate<E> predicate)`
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
Review Comment:
We should add UTs for checking the behaviour of all methods changed as part
of this async initialisation.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestPrefetch.java:
##########
@@ -364,16 +366,20 @@ public void testPrefetchWithDelay() throws Exception {
// Wait for 20 seconds, no thread should start prefetch
Thread.sleep(20000);
assertFalse("Prefetch threads should not be running at this point",
reader.prefetchStarted());
- while (!reader.prefetchStarted()) {
- assertTrue("Prefetch delay has not been expired yet",
- getElapsedTime(startTime) < PrefetchExecutor.getPrefetchDelay());
- }
- if (reader.prefetchStarted()) {
- // Added some delay as we have started the timer a bit late.
+ long timeout = 10000;
+ while (!reader.prefetchStarted() && !reader.prefetchComplete()) {
+ // Wait until the prefetch is triggered.
Thread.sleep(500);
- assertTrue("Prefetch should start post configured delay",
- getElapsedTime(startTime) > PrefetchExecutor.getPrefetchDelay());
+ if (timeout <= 0) break;
+ timeout -= 500;
}
+ assertTrue(reader.prefetchStarted() || reader.prefetchComplete());
+
+ // Added some delay as we have started the timer a bit late.
+ Thread.sleep(500);
Review Comment:
Why do we need this extra sleep after the loop above?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java:
##########
@@ -226,10 +226,19 @@ public void testHeapSizeChanges() throws Exception {
public static void waitUntilFlushedToBucket(BucketCache cache, BlockCacheKey
cacheKey)
throws InterruptedException {
- while (!cache.backingMap.containsKey(cacheKey) ||
cache.ramCache.containsKey(cacheKey)) {
- Thread.sleep(100);
+ long timeout = 120000;
+ try {
+ while (!cache.backingMap.containsKey(cacheKey) ||
cache.ramCache.containsKey(cacheKey)) {
+ Thread.sleep(100);
+ if (timeout <= 0) {
+ break;
+ }
+ timeout -= 100;
+ }
+ } finally {
+ Thread.sleep(1000);
Review Comment:
Use `Waiter.waitFor(Configuration conf, long timeout, long interval,
Predicate<E> predicate)`
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestRecoveryPersistentBucketCache.java:
##########
@@ -143,7 +147,16 @@ public void testBucketCacheEvictByHFileAfterRecovery()
throws Exception {
private void waitUntilFlushedToBucket(BucketCache cache, BlockCacheKey
cacheKey)
throws InterruptedException {
- while (!cache.backingMap.containsKey(cacheKey) ||
cache.ramCache.containsKey(cacheKey)) {
+ try {
+ long timeout = 120000;
+ while (!cache.backingMap.containsKey(cacheKey) ||
cache.ramCache.containsKey(cacheKey)) {
+ Thread.sleep(100);
+ if (timeout <= 0) {
+ break;
+ }
+ timeout -= 100;
+ }
Review Comment:
Use `Waiter.waitFor(Configuration conf, long timeout, long interval,
Predicate<E> predicate)`
--
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]
