diqiu50 commented on code in PR #12417:
URL: https://github.com/apache/gravitino/pull/12417#discussion_r3759302499
##########
core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java:
##########
@@ -83,6 +85,13 @@ public class CaffeineEntityCache extends BaseEntityCache {
private static final Logger LOG =
LoggerFactory.getLogger(CaffeineEntityCache.class.getName());
+ /**
+ * Characters that can separate a cached entity's identifier from a
descendant's. See {@link
+ * #invalidateHierarchy(EntityCacheKey)} for why both are needed.
+ */
+ private static final List<String> CHILD_KEY_BOUNDARIES =
+ ImmutableList.of(".", HierarchicalSchemaUtil.physicalSeparator());
Review Comment:
Do we need to use `HierarchicalSchemaUtil.schemaSeparator()`
##########
core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java:
##########
@@ -267,22 +276,36 @@ protected void invalidateExpiredItem(EntityCacheKey key) {
/**
* Removes the entry for the given key and all cached descendant entries.
Descendants are found
- * through the prefix index: every child identifier starts with {@code
parent identifier + "."},
- * so the scan is exact for children and never matches siblings sharing a
name prefix (e.g. {@code
- * catalog1} vs {@code catalog10}).
+ * through the prefix index, scanning once per child boundary:
+ *
+ * <ul>
+ * <li>{@code "."} separates {@link org.apache.gravitino.NameIdentifier}
levels, so it matches
Review Comment:
FQN
##########
core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheInvalidation.java:
##########
@@ -155,6 +167,108 @@ void testGetIfPresentReturnsCachedEntity() {
cache.getIfPresent(catalog.nameIdentifier(),
Entity.EntityType.SCHEMA).isEmpty());
}
+ @Test
+ void testInvalidateHierarchicalSchemaCascadesToNestedSchemas() {
+ // A HierarchicalSchema nests inside a single name level, joined by the
physical separator,
+ // so "raw:events:2024" is a child of "raw:events" without adding a
NameIdentifier level.
+ String parentName = hierarchicalName("raw", "events");
+ String childName = hierarchicalName("raw", "events", "2024");
+
+ SchemaEntity parent = TestUtil.getTestSchemaEntity(2L, parentName,
CATALOG_NS, "cmt");
+ SchemaEntity child = TestUtil.getTestSchemaEntity(3L, childName,
CATALOG_NS, "cmt");
+ TableEntity tableInParent =
+ TestUtil.getTestTableEntity(4L, "t_parent",
schemaNamespace(parentName));
+ TableEntity tableInChild =
+ TestUtil.getTestTableEntity(5L, "t_child", schemaNamespace(childName));
+
+ cache.put(parent);
+ cache.put(child);
+ cache.put(tableInParent);
+ cache.put(tableInChild);
+ Assertions.assertEquals(4, cache.size());
+
+ cache.invalidate(parent.nameIdentifier(), Entity.EntityType.SCHEMA);
+
+ Assertions.assertFalse(cache.contains(parent.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertFalse(cache.contains(tableInParent.nameIdentifier(),
Entity.EntityType.TABLE));
+ Assertions.assertFalse(cache.contains(child.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertFalse(cache.contains(tableInChild.nameIdentifier(),
Entity.EntityType.TABLE));
+ Assertions.assertEquals(0, cache.size());
+ }
+
+ @Test
+ void testInvalidateHierarchicalSchemaCascadesToAnyDepth() {
+ String level1 = hierarchicalName("raw");
+ String level2 = hierarchicalName("raw", "events");
+ String level3 = hierarchicalName("raw", "events", "2024");
+ String level4 = hierarchicalName("raw", "events", "2024", "q1");
+
+ cache.put(TestUtil.getTestSchemaEntity(2L, level1, CATALOG_NS, "cmt"));
+ cache.put(TestUtil.getTestSchemaEntity(3L, level2, CATALOG_NS, "cmt"));
+ cache.put(TestUtil.getTestSchemaEntity(4L, level3, CATALOG_NS, "cmt"));
+ SchemaEntity deepest = TestUtil.getTestSchemaEntity(5L, level4,
CATALOG_NS, "cmt");
+ cache.put(deepest);
+ TableEntity deepestTable = TestUtil.getTestTableEntity(6L, "t_deep",
schemaNamespace(level4));
+ cache.put(deepestTable);
+ Assertions.assertEquals(5, cache.size());
+
+ cache.invalidate(
+ TestUtil.getTestSchemaEntity(2L, level1, CATALOG_NS,
"cmt").nameIdentifier(),
+ Entity.EntityType.SCHEMA);
+
+ Assertions.assertFalse(cache.contains(deepest.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertFalse(cache.contains(deepestTable.nameIdentifier(),
Entity.EntityType.TABLE));
+ Assertions.assertEquals(0, cache.size());
+ }
+
+ @Test
+ void testInvalidateHierarchicalSchemaDoesNotTouchSiblings() {
+ // Guards against over-matching: "raw:events2" is a sibling of
"raw:events", not a descendant,
+ // exactly like the catalog1 / catalog10 case the "." boundary already
protects against.
+ String target = hierarchicalName("raw", "events");
+ String sibling = hierarchicalName("raw", "events2");
+ String siblingOfParent = hierarchicalName("raw2", "events");
+
+ SchemaEntity targetSchema = TestUtil.getTestSchemaEntity(2L, target,
CATALOG_NS, "cmt");
+ SchemaEntity siblingSchema = TestUtil.getTestSchemaEntity(3L, sibling,
CATALOG_NS, "cmt");
+ SchemaEntity otherBranch = TestUtil.getTestSchemaEntity(4L,
siblingOfParent, CATALOG_NS, "cmt");
+ TableEntity siblingTable =
+ TestUtil.getTestTableEntity(5L, "t_sibling", schemaNamespace(sibling));
+
+ cache.put(targetSchema);
+ cache.put(siblingSchema);
+ cache.put(otherBranch);
+ cache.put(siblingTable);
+
+ cache.invalidate(targetSchema.nameIdentifier(), Entity.EntityType.SCHEMA);
+
+ Assertions.assertFalse(cache.contains(targetSchema.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertTrue(cache.contains(siblingSchema.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertTrue(cache.contains(otherBranch.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertTrue(cache.contains(siblingTable.nameIdentifier(),
Entity.EntityType.TABLE));
+ Assertions.assertEquals(3, cache.size());
+ }
+
+ @Test
+ void testInvalidateCatalogCascadesToHierarchicalSchemas() {
+ CatalogEntity catalog =
+ TestUtil.getTestCatalogEntity(1L, "catalog1",
Namespace.of("metalake"), "hive", "cmt");
+ String nested = hierarchicalName("raw", "events", "2024");
+ SchemaEntity schema = TestUtil.getTestSchemaEntity(2L, nested, CATALOG_NS,
"cmt");
+ TableEntity table = TestUtil.getTestTableEntity(3L, "t1",
schemaNamespace(nested));
+
+ cache.put(catalog);
+ cache.put(schema);
+ cache.put(table);
+ Assertions.assertEquals(3, cache.size());
+
+ cache.invalidate(catalog.nameIdentifier(), Entity.EntityType.CATALOG);
+
+ Assertions.assertFalse(cache.contains(schema.nameIdentifier(),
Entity.EntityType.SCHEMA));
+ Assertions.assertFalse(cache.contains(table.nameIdentifier(),
Entity.EntityType.TABLE));
+ Assertions.assertEquals(0, cache.size());
+ }
+
Review Comment:
We need to add a test case to use `RelationalEntityStore`
We need to add a test case that uses a non-default separator
--
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]
