JoegenUSTC commented on PR #11846:
URL: https://github.com/apache/gravitino/pull/11846#issuecomment-5067055567

   @diqiu50 @yuqi1129
   
   Rebased on `main` and integrated with ClassLoaderPool (#10480). A few things 
worth noting.
   
   **ClassLoaderPool covers most cases — but not all.**
   It implements @diqiu50's Approach 2 (reference counting), which is the right 
long-term fix. However:
   - `classloader.sharing.enabled=false` (non-pooled mode) has no 
reference-counting protection — close() still tears down the classloader on 
eviction.
   - Even in pooled mode, the last release still closes the classloader, so a 
Capability object that escapes the boundary can still trigger the `$1` 
synthetic-class `NoClassDefFoundError`.
   
   The if/else fix (4 lines per catalog) is the root cause removal and remains 
valid regardless of pooling.
   
   **What changed in this update:**
   
   1. **Integrated with ClassLoaderPool** — `doWithCapabilityOps` checks the 
existing `closed` flag; `close()` reuses main's pooled/non-pooled logic; no 
conflict with ClassLoaderPool's lifecycle.
   2. **Migrated new dispatchers** — `FunctionNormalizeDispatcher`, 
`ModelNormalizeDispatcher`, `SchemaNormalizeDispatcher`, 
`TopicNormalizeDispatcher` migrated from `getCapability` → `withCapability`.
   3. **`createTable` optimization** — bundled 6 separate `withCapability` 
calls into one via `NormalizedCreateArgs`, reducing TCCL switches from 14 back 
to the pre-PR baseline of 4 (per @diqiu50's feedback).
   
   **Flexible scope.** If you'd prefer a smaller PR, I can trim to just the 
if/else fix + `CatalogWrapperClosedException` (~60 lines across 4 files) and 
defer the dispatcher migration. Happy to go either way — just let me know the 
preferred direction.
   
   Latest commit: 
[e0af8a3db](https://github.com/JoegenUSTC/gravitino/commit/e0af8a3db)


-- 
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]

Reply via email to