JoegenUSTC commented on PR #11846: URL: https://github.com/apache/gravitino/pull/11846#issuecomment-4965033632
> Thanks for the thorough root-cause analysis — the three-part approach is sound. I left two inline comments about (1) the retry-once logic potentially replaying non-idempotent operations, and (2) whole dispatcher calls now running inside the synchronized classloader boundary. Both can be resolved together by keeping `fn` limited to pure normalization and moving the dispatcher call back outside the boundary. @yuqi1129 Thanks for the thorough review — both issues are now addressed in a new commit (`71d0bb2`). **Comment 1 — retry logic in `withCapability`** Added a package-private `CatalogWrapperClosedException extends IllegalStateException` thrown exclusively at the entry of `doWithCapabilityOps` (before `fn` has a chance to run). `withCapability` now retries only on this dedicated type, so any `IllegalStateException` raised by `fn` itself mid-execution is never silently retried. The `isClosed()` heuristic is gone. **Comment 2 — dispatcher calls inside the synchronized boundary** Restricted `fn` to pure normalization (CPU-only, no external I/O) across all affected sites: - `TableNormalizeDispatcher.createTable` — 6 normalization steps, each in its own `withCapability` call; `dispatcher.createTable` is outside. - `TableNormalizeDispatcher.alterTable` / `FilesetNormalizeDispatcher.alterFileset` / `ViewNormalizeDispatcher.alterView` — each uses a `Pair` to bundle the two normalized values (matching your suggested pattern), then calls the dispatcher outside. - `PartitionNormalizeDispatcher` (all 6 methods) — `Pair`-based normalization inside; dispatcher outside. For `listPartitionNames` and `listPartitions` that also need output normalization, a second lightweight `withCapability` call is used after the RPC completes. Lock hold time drops back to O(normalization) — microseconds — eliminating per-catalog serialization and the eviction-thread stall risk. And since `fn` is now a pure function, the retry is inherently safe regardless of the exception type. -- 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]
