yuqi1129 opened a new pull request, #12417:
URL: https://github.com/apache/gravitino/pull/12417
### What changes were proposed in this pull request?
Make `CaffeineEntityCache.invalidateHierarchy` scan the prefix index once
per child boundary instead of only for `"."`.
- Add `CHILD_KEY_BOUNDARIES` = `{".",
HierarchicalSchemaUtil.physicalSeparator()}` and iterate it when collecting
descendants.
- Correct the `invalidateHierarchy` javadoc, which asserted the incorrect
invariant that every child identifier starts with `parent identifier + "."`.
- Add cascade tests covering hierarchical schema names, including sibling
and depth cases.
### Why are the changes needed?
`invalidateHierarchy` found cached descendants with a single prefix scan:
```java
String childPrefix = key.identifier().toString() + ".";
```
This assumes every child identifier starts with the parent identifier
followed by `.`. A `HierarchicalSchema` breaks that assumption: its nested
levels are not extra `NameIdentifier` levels, they are joined **inside a single
name level** by the physical separator (`Configs.SCHEMA_SEPARATOR` forbids `.`
as the external separator). So `raw:events:2024` is stored as one schema name
whose identifier continues past `raw:events` with the physical separator, not
with `.`.
Dropping or renaming `raw:events` therefore left `raw:events:2024`, any
deeper nesting, and every table/view/fileset/topic below them in the cache
until TTL:
```
--- after invalidate(raw:events schema) ---
parent cached = false (expected)
tblInParent cached = false (expected)
child cached = true <-- stale
tblInChild cached = true <-- stale
```
This is not multi-node specific. `RelationalEntityStore#delete` and the
rename paths invalidate through the same method, so the node performing the
mutation keeps stale descendants too; it reproduces on a single node.
The trailing separator itself was correct and is kept — it is the guard that
stops `catalog1` from matching `catalog10`. The defect was that there is more
than one valid child boundary and only one was handled. Because the radix index
matches on whole key strings, the added pass reaches descendants at any depth,
so no recursion is needed.
Fix: #12416
### Does this PR introduce _any_ user-facing change?
No. No public API or configuration property changes. Cascading invalidation
of hierarchical schemas now behaves as already documented.
### How was this patch tested?
Added four cases to `TestCaffeineEntityCacheInvalidation`. The first two
fail before this change and pass after; the last two pass both before and after
and are regression guards proving the fix does not over-invalidate:
| Test | Before |
| --- | --- |
| `testInvalidateHierarchicalSchemaCascadesToNestedSchemas` | fails |
| `testInvalidateHierarchicalSchemaCascadesToAnyDepth` (four levels) | fails
|
| `testInvalidateHierarchicalSchemaDoesNotTouchSiblings` (`raw:events` vs
`raw:events2`) | passes |
| `testInvalidateCatalogCascadesToHierarchicalSchemas` | passes |
Commands:
- `./gradlew :core:test --tests
"org.apache.gravitino.cache.TestCaffeineEntityCacheInvalidation" -PskipITs
-PskipDockerTests`
- `./gradlew :core:test -PskipITs -PskipDockerTests`
- `./gradlew :core:spotlessCheck -PskipITs -PskipDockerTests`
- `./gradlew :core:javadoc -PskipITs -PskipDockerTests`
One case from the issue is intentionally not included here: a cross-node
assertion in `TestEntityCacheCrossNodeInvalidation`, which does not exist on
`main` yet (it is introduced by #12374). Cross-node replay funnels into the
same `cache.invalidate` → `invalidateHierarchy` entry point that these unit
tests cover, so the mechanism is exercised; the cross-node case is worth adding
once #12374 lands.
--
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]
