yuqi1129 commented on PR #11846: URL: https://github.com/apache/gravitino/pull/11846#issuecomment-4935625013
Thanks for the work here. After looking through the failure path again, I think this PR may be solving the problem at too broad a level. My understanding is: 1. Calling an old catalog object after its wrapper has been closed is expected to fail. The more important issue is that normal request paths should not keep using plugin-loaded executable objects after they have escaped the `CatalogWrapper` boundary. 2. The immediate trigger for the observed `NoClassDefFoundError` is the `switch`-on-enum in catalog `Capability` implementations, such as Hive and Glue. That generates a synthetic `$1` switch-map class, which may be loaded lazily. Replacing these with `if/else` avoids that synthetic class and directly removes the current trigger. We should also add tests to ensure the `$1` class is not generated. 3. The root boundary fix should be to stop letting `Capability` escape. We should deprecate or narrow `CapabilityHelpers.getCapability()` and `CatalogWrapper.capabilities()`, and require capability logic to go through a wrapper-managed entry point such as `withCapability(ident, fn)`. That method should load the wrapper, enter the catalog classloader, execute the lambda, and reload/retry if it detects a closed stale wrapper. So I do not think we should add a `ReadWriteLock` around every `doWithXxxOps()` method. That makes the fix much broader and still does not fully close the boundary, because `catalog()` / `capabilities()` remain raw escape paths and some callers can still bypass the lock. In short, I think the right direction is: - short-term: remove `switch`-on-enum from all catalog `Capability` implementations, especially Hive and Glue; - root fix: ensure `Capability` methods are only invoked through a wrapper/classloader-bounded helper; - avoid turning this into a full catalog-operation lifecycle lock unless we have a separate issue proving catalog operations themselves need that behavior. -- 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]
