yuqi1129 opened a new pull request, #12404: URL: https://github.com/apache/gravitino/pull/12404
### What changes were proposed in this pull request? - `CatalogManager.CatalogWrapper` now counts active operations: `tryAcquire()` takes a lease, `release()` returns it, and `retire()` marks the wrapper unusable for new leases without tearing anything down. The catalog and the `IsolatedClassLoader` are cleaned up exactly once, when the wrapper is retired **and** its last lease has been released (cleanup runs outside the lock so a slow catalog close cannot block new lease attempts). - The catalog cache removal listener calls `retire()` instead of `close()`; `close()` is kept as an alias of `retire()` for exclusive owners such as `testConnection`. - New `CatalogLease` (`AutoCloseable`) obtained from `CatalogManager.acquireCatalogLease(ident)`, which reloads a fresh wrapper (bounded retries) when the cached one has already retired. - All production uses migrated to the lease API: `loadCatalog`, `enableCatalog`, `disableCatalog`, `alterCatalog`, `dropCatalog`, `getResolvedProperties`, `OperationDispatcher.doWithTable/doWithCatalog`, `CapabilityHelpers.getCapability` and `IcebergCleanupHelper.catalogId`. - `loadCatalogAndWrap` stays for compatibility (documented as unleased) and now detects a stale cache entry via `isRetired()` instead of `catalog() != null`. ### Why are the changes needed? `CatalogWrapper.close()` was invoked from the cache removal listener, which Caffeine runs asynchronously and outside the local `TreeLock`. A cache expiry, a remote change-log invalidation, or a drop could therefore close the catalog, null its reference and release the pooled ClassLoader while another thread was still using the wrapper, surfacing as `NullPointerException`, `NoClassDefFoundError`, or errors from prematurely closed catalog resources. Fix: #12403 ### Does this PR introduce _any_ user-facing change? No. New internal APIs only (`CatalogLease`, `CatalogWrapper#tryAcquire/release/retire/isRetired`); no configuration or REST API change. ### How was this patch tested? - New `TestCatalogWrapperLease` (8 tests): cache expiry, explicit invalidation, remote change-log invalidation, drop, retired-wrapper reload, exactly-once ClassLoader cleanup (verified through the shared pool reference count), release-without-acquire, and a two-thread repro where the cache is invalidated while an operation holds the wrapper. - Negative check: making cleanup eager again (the old behaviour) fails 5 of the 8 tests, including the concurrent repro. - `./gradlew :core:test :server:test :iceberg:iceberg-rest-server:test -PskipITs` all pass (core: 1618 tests); repo-wide `compileJava`/`compileTestJava`, `spotlessCheck` and `javadoc` are clean. -- 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]
