pithecuse527 commented on issue #13695: URL: https://github.com/apache/gravitino/issues/13695#issuecomment-6137186089
@yuqi1129 just asking out of curiosity... It seems that the metalake's own key is the only exception where the resource type comes after the name (e.g., `<ml>␟METALAKE␟`, as shown in https://github.com/apache/gravitino/blob/10d51bd966af6cf4d3cdebc6290de4def9f0b35c/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationCacheKeys.java#L37-L43 Other resource keys follow the pattern `<ml>␟<TYPE>␟<name>…`, such as `<ml>␟CATALOG␟<ctlg>␟` or `<ml>␟TAG␟<tag>␟`... in other words, the usual ordering is `<TYPE>␟<name>`, whereas the metalake's own key follows the reverse order, `<name>␟<TYPE>` (i.e., `<ml>␟METALAKE␟`). So... my understanding is that all resource types follow the pattern `<ml>␟<TYPE>␟<name>…`, except for the metalake itself, which uses `<ml>␟METALAKE␟`. Would it be reasonable to remove the `METALAKE` token so that it restores the "Parent key = prefix of child key" rule? (as it was before [#10996](https://github.com/apache/gravitino/pull/10996)) As far as I can tell, `<metalake>␟` on its own wouldn't conflict with any other keys. I agree that the proposed fix works, but it seems that the prefix invalidation can no longer use original `metadataIdCacheKey` directly...? For example, ```java // JcasbinAuthorizer.java::handleEntityNameIdMappingChange ... if (hasNestedMetadataObjects(type)) { metadataIdCache.invalidateByPrefix( metadataIdInvalidationPrefix(metalake, metadataObject)); // ← calling new fucntion } else { metadataIdCache.invalidate(metadataIdCacheKey(metalake, metadataObject)); } // JcasbinAuthorizationCacheKeys.java // new helper function static String metadataIdInvalidationPrefix(String metalake, MetadataObject metadataObject) { if (metadataObject.type() == MetadataObject.Type.METALAKE) { return metalakeScopePrefix(metalake); } return metadataIdCacheKey(metalake, metadataObject); } ``` This would introduce one more complication, and any new invalidation path would also have to use that separate prefix instead of `metadataIdCacheKey`. However... dropping the `METALAKE` token would let them keep using `metadataIdCacheKey` so it is simple. Please let me know if i missed something about your proposed fix and Intention. -- 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]
