Copilot commented on code in PR #12445:
URL: https://github.com/apache/gravitino/pull/12445#discussion_r3771777935
##########
server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangeListener.java:
##########
@@ -275,7 +280,17 @@ public synchronized void
onEntityChange(List<EntityChangeRecord> changes) {
leafKeys.add(cacheKey);
}
}
- invalidateCoalescedKeys(containerPrefixes, leafKeys);
+ try {
+ invalidateCoalescedKeys(containerPrefixes, leafKeys);
+ } catch (RuntimeException e) {
+ LOG.error(
+ "Failed to invalidate {} prefix(es) and {} leaf key(s) from the
entity change log, "
+ + "clearing the whole metadata id cache to stay coherent",
+ containerPrefixes.size(),
+ leafKeys.size(),
+ e);
+ metadataIdCache.invalidateAll();
+ }
Review Comment:
`metadataIdCache.invalidateAll()` is called outside the cache's
`runInvalidationBatch` recovery lock. If `invalidateCoalescedKeys` fails after
evicting some prefixes/keys, the batch lock is released before the clear, which
can expose a half-applied invalidation state to concurrent authorization reads.
Consider performing the clear while still holding the invalidation lock, and
only falling back to an unlocked clear when the batch mechanism itself fails.
--
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]
