mnpoonia commented on code in PR #8456:
URL: https://github.com/apache/hbase/pull/8456#discussion_r3530551672
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestRecoveryPersistentBucketCache.java:
##########
@@ -251,9 +253,12 @@ public void
testBucketCacheRecoveryWithAllocationInconsistencies() throws Except
BucketCache newBucketCache = new BucketCache("file:" + testDir +
"/bucket.cache", 36 * 1024,
8192, bucketSizes, writeThreads, writerQLen, testDir +
"/bucket.persistence",
DEFAULT_ERROR_TOLERATION_DURATION, conf);
- while (!newBucketCache.getBackingMapValidated().get()) {
- Thread.sleep(10);
- }
+ // backingMapValidated is set inside retrieveFromFile(), but
isCacheEnabled() (which
+ // getBlock() checks) is set to true only in the finally block that
follows. Waiting on
+ // backingMapValidated alone leaves a race window; await both conditions
together.
+ await().atMost(Duration.ofSeconds(30)).pollDelay(Duration.ofMillis(10))
+ .pollInterval(Duration.ofMillis(10)).until(
Review Comment:
You're right — Awaitility's pollDelay defaults to the same value as
pollInterval when not explicitly set. So we could have written just:
await().atMost(Duration.ofSeconds(30))
.pollInterval(Duration.ofMillis(10)).until(...)
and it would have the same behavior (10ms initial delay, 10ms between
polls). Setting both explicitly just makes the intent clearer. What do you
think.
--
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]
