yuqi1129 commented on code in PR #13152:
URL: https://github.com/apache/gravitino/pull/13152#discussion_r4060621836
##########
core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java:
##########
@@ -346,6 +348,50 @@ void
testInvalidateLeafDoesNotEvictSameNameEntityOfAnotherType() {
Assertions.assertTrue(cache.contains(topic.nameIdentifier(),
Entity.EntityType.TOPIC));
}
+ @Test
+ void testExpiredRemovalCallbackAfterReinsertKeepsIndexEntry() throws
Exception {
+ // Deterministic regression for the removal-listener race: the
asynchronous callback of an
+ // expired/evicted entry must not delete the index entry of a reinserted
entry with the same
+ // key, otherwise later parent-level invalidation can no longer discover
the child.
+ Config config = new Config(false) {};
+ // 50 ms TTL so the first entry expires quickly while the test stays
deterministic.
+ config.set(Configs.CACHE_EXPIRATION_TIME, 50L);
+ cache = new CaffeineEntityCache(config);
+
+ CatalogEntity catalog =
+ TestUtil.getTestCatalogEntity(1L, "catalog1",
Namespace.of("metalake"), "hive", "cmt");
+ SchemaEntity schema =
+ TestUtil.getTestSchemaEntity(2L, "schema1", Namespace.of("metalake",
"catalog1"), "cmt");
+ TableEntity table =
+ TestUtil.getTestTableEntity(3L, "table1", Namespace.of("metalake",
"catalog1", "schema1"));
+ cache.put(catalog);
+ cache.put(schema);
+ cache.put(table);
+
+ // Wait past the TTL so the table entry expires. Caffeine's removal
listener runs
+ // asynchronously on the cache's executor (CLEANUP_EXECUTOR), reinsert the
entity first and
+ // give the delayed callback a chance to run. Use Awaitility for a
deterministic wait.
+ Awaitility.await()
+ .atMost(Duration.ofSeconds(5))
+ .untilAsserted(
+ () ->
+ Assertions.assertNull(
+ cache.getCacheData().getIfPresent(
+ EntityCacheKey.of(table.nameIdentifier(),
Entity.EntityType.TABLE))));
+
+ // Reinsert the table: cacheData now holds a fresh entry and cacheIndex
holds its key again.
+ cache.put(table);
Review Comment:
The `Awaitility` check only proves the old entry expired; the cleanup
callback may already have run before this `put`, so the pre-fix implementation
can pass this test. Since this test is in the same package, a simple
deterministic regression is to reinsert the table and then call
`cache.invalidateExpiredItem(tableKey)` to model the delayed old callback,
before invalidating the catalog. That exact ordering makes the old
unconditional `cacheIndex.remove` fail while the new implementation passes.
Alternatively, inject a controllable cleanup executor and release its callback
after this `put`.
--
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]
