kbendick commented on a change in pull request #3801:
URL: https://github.com/apache/iceberg/pull/3801#discussion_r774843656
##########
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:
Side Note: Since you're introducing `Awaitility` (which in general I
think is a good idea), we should update the rest of the tests to use
`Awaitility` and remove the call to `cleanUp` in the `TestableCachingCatalog`,
which was added to handle the async expiration.
https://github.com/apache/iceberg/blob/9c3e340cd67699c3d2499762d794ab1bc1ee7f45/core/src/test/java/org/apache/iceberg/TestableCachingCatalog.java#L46-L51
This larger refactor to using `Awaitility` entirely should be done in a
separate PR though, as this fix is important and I'd like to get this PR in as
soon as possible so people can use the updated snapshot and we'll continue to
see if the race condition is fixed.
The use of awaitility is fine here though, but I was curious why it was
needed so I went looking through the `Caffeine` issues and update notes and
found the likely cause. Updating Caffeine to 2.8.5 would make this change not
needed.
All that said, I think the use of `Awaitility` here is fine. =)
Thanks again for all your work on this!
--
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]
