kbendick commented on a change in pull request #3801:
URL: https://github.com/apache/iceberg/pull/3801#discussion_r774845284
##########
File path: core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java
##########
@@ -252,9 +254,9 @@ public void
testCacheExpirationEagerlyRemovesMetadataTables() throws IOException
ticker.advance(HALF_OF_EXPIRATION);
Assertions.assertThat(catalog.cache().asMap()).doesNotContainKey(tableIdent);
- Arrays.stream(metadataTables(tableIdent)).forEach(metadataTable ->
- Assert.assertFalse("When a data table expires, its metadata tables
should expire regardless of age",
- catalog.cache().asMap().containsKey(metadataTable)));
+ // Removal of metadata tables from cache is async, use awaitility
+ await().untilAsserted(() ->
+
Assertions.assertThat(catalog.cache().asMap()).doesNotContainKeys(metadataTables(tableIdent)));
Review comment:
Actually, this will balloon up the PR and this is an important fix.
I'd say let's move everything in this test suite to `awaitility` and see if
we can remove the above call in a separate PR (though the present use of
awaitlity is fine). The priority should be on preventing the deadlock from
occurring :)
That way we can review this change and get it out without having to review
the much larger diff that will come from adding `awaitility` to a lot of this
file.
If you don't want to do the separate PR, let me know and I'll take care of
it. I'll leave that up to you. But it should definitely be a separate PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
