sunyuhan1998 commented on code in PR #11702:
URL: https://github.com/apache/gravitino/pull/11702#discussion_r3427155754
##########
core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java:
##########
@@ -195,6 +195,26 @@ public boolean invalidate(
});
}
+ /** {@inheritDoc} */
+ @Override
+ public boolean invalidateRelationEntry(
+ NameIdentifier ident, Entity.EntityType type,
SupportsRelationOperations.Type relType) {
+ checkArguments(ident, type, relType);
+ EntityCacheRelationKey key = EntityCacheRelationKey.of(ident, type,
relType);
+ return segmentedLock.withLock(
+ key,
+ () -> {
+ // Drop only the cached relation result and its index entry. Do NOT
cascade through the
+ // reverse index: it is shared across entities (e.g. all roles bound
to one metadata
+ // object), and evicting it here would drop other entities'
reverse-index mappings. An
+ // explicit cacheData.invalidate does not trigger the expiry
listener, so the reverse
+ // index is left intact.
+ cacheData.invalidate(key);
+ cacheIndex.remove(key.toString());
+ return true;
Review Comment:
Fixed in the latest push: `invalidateRelationEntry` now also calls
`reverseIndex.remove(key)` to clean up this relation key's own reverse-index
bookkeeping (`entityToReverseIndexMap` + the `reverseIndex` references pointing
at it) — no BFS cascade, so other entities' mappings are preserved.
On the "leak over time" severity: it's bounded in practice (one metadata
object maps to a single relation key, rebuilt/merged on the next read), but
cleaning it up is clearly more correct, so I adopted it.
On `cacheData.asMap().remove` for an accurate boolean: I kept
`cacheData.invalidate` to stay consistent with
`invalidate(ident,type,relType)`, which also returns `true` unconditionally and
whose callers don't inspect the return value.
--
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]
