yuqi1129 opened a new issue, #12416:
URL: https://github.com/apache/gravitino/issues/12416
### Version
main branch
### Describe what's wrong
`CaffeineEntityCache.invalidateHierarchy` finds cached descendants with a
single prefix scan that assumes every child identifier starts with `parent
identifier + "."`:
```java
String childPrefix = key.identifier().toString() + ".";
```
That assumption does not hold for `HierarchicalSchema`. Nested schema levels
are not extra `NameIdentifier` levels — they are joined **inside one level**
with the physical separator `\u0001`
(`HierarchicalSchemaUtil.PHYSICAL_SEPARATOR`; `Configs.SCHEMA_SEPARATOR`
forbids `.` as the external separator). So invalidating a hierarchical schema
leaves every nested schema below it, and all of their
tables/views/filesets/topics, in the cache until TTL.
The trailing `"."` itself is correct and necessary — it is the guard that
stops `catalog1` from matching `catalog10`. The defect is that there is more
than one valid child boundary, and only one of them is handled.
Impact:
- **Not limited to remote nodes.** `RelationalEntityStore#delete`
invalidates through the same path, so the node performing the drop also keeps
stale descendants. This reproduces on a single-node deployment.
- **Rename is affected too**, via the same `cache.invalidate(srcIdentifier,
srcType)` call.
- **Any depth is affected** — `raw:events:2024:q1` and its tables are missed
as well.
- The only backstop is the cache TTL (minutes to hours), so a stale read can
be served long after the entity is gone.
### Error message and/or stacktrace
No exception is thrown — the invalidation silently under-matches, which is
what makes this hard to notice. Observed cache state after invalidating the
parent schema:
```
parent key = ml1.cat1.raw<SEP>events:SCHEMA
child key = ml1.cat1.raw<SEP>events<SEP>2024:SCHEMA
tblInParent key = ml1.cat1.raw<SEP>events.t_parent:TABLE
tblInChild key = ml1.cat1.raw<SEP>events<SEP>2024.t_child:TABLE
--- after invalidate(parent schema) ---
parent cached = false (expected)
tblInParent cached = false (expected)
child cached = true <-- stale
tblInChild cached = true <-- stale
size = 2
```
`<SEP>` is the physical separator `\u0001`.
### How to reproduce
+ Version: main branch
+ Cache enabled (`gravitino.cache.enabled=true`, default `caffeine`), a
catalog supporting `HierarchicalSchema`
Steps:
1. Create a hierarchical schema `raw:events` under `ml1.cat1`, and a nested
schema `raw:events:2024` under it.
2. Create a table in each schema.
3. Load all four entities so they are cached.
4. Drop (or rename) `raw:events`.
5. The nested schema `raw:events:2024` and its table are still cached.
Equivalent unit-level reproduction against `CaffeineEntityCache`:
```java
CaffeineEntityCache cache = new CaffeineEntityCache(new Config() {});
String sep = HierarchicalSchemaUtil.physicalSeparator();
Namespace catNs = Namespace.of("ml1", "cat1");
String parentName = "raw" + sep + "events";
String childName = "raw" + sep + "events" + sep + "2024";
cache.put(TestUtil.getTestSchemaEntity(2L, parentName, catNs, "cmt"));
cache.put(TestUtil.getTestSchemaEntity(3L, childName, catNs, "cmt"));
cache.put(TestUtil.getTestTableEntity(4L, "t_parent", Namespace.of("ml1",
"cat1", parentName)));
cache.put(TestUtil.getTestTableEntity(5L, "t_child", Namespace.of("ml1",
"cat1", childName)));
cache.invalidate(NameIdentifier.of("ml1", "cat1", parentName),
Entity.EntityType.SCHEMA);
// fails: the nested schema and its table are still cached, cache.size() == 2
```
### Additional context
There is currently no test in
`core/src/test/java/org/apache/gravitino/cache/` that exercises a hierarchical
schema name, which is why this was not caught. The javadoc on
`invalidateHierarchy` also states the incorrect invariant explicitly ("every
child identifier starts with `parent identifier + "."`") and should be
corrected along with the fix.
Suggested fix — scan for both child boundaries, which preserves the existing
sibling guard:
```java
String base = key.identifier().toString();
for (String boundary : new String[] {".",
HierarchicalSchemaUtil.physicalSeparator()}) {
for (EntityCacheKey childKey :
Lists.newArrayList(cacheIndex.getValuesForKeysStartingWith(base +
boundary))) {
cacheData.invalidate(childKey);
cacheIndex.remove(childKey.toString());
}
}
```
Because the radix index matches on the whole key string, the `\u0001` pass
collects descendants at any depth, so no recursion is needed. Local delete,
rename, and cross-node change-log replay all funnel through
`invalidateHierarchy`, so this single change covers all three paths and needs
no change-log protocol change.
Tests worth adding: drop a hierarchical schema cascades to nested schemas
and their tables; three-level nesting; **siblings are not over-invalidated**
(`raw:events` vs `raw:events2`); dropping a catalog clears hierarchical schemas
beneath it; and a cross-node case in `TestEntityCacheCrossNodeInvalidation`
confirming one root DROP row expands correctly on a peer node.
--
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]
