[#12986] fix(catalog): Release the ClassLoader of a dropped catalog - #12987
Merged
Conversation
…alog Dropping or altering a hive, lakehouse-iceberg, lakehouse-paimon or S3-backed fileset catalog left its isolated ClassLoader reachable, so its classes stayed in Metaspace for the life of the process. Since an alter rebuilds the ClassLoader, the loss accumulated: five alters of a Hive catalog cost about 48 MB that was never returned. Three retention paths, each traced from a heap dump back to a GC root: - commons-logging's Log4jApiLogFactory registers a LogAdapter with the server's shared Log4j LoggerContext, and LogFactory.release leaves that registration in place. Remove listeners belonging to the loader. - Jackson-style caches park a SoftReference in a ThreadLocal. clearThreadLocalMap compared the value's own class, but the value is a bootstrap SoftReference and only its referent names the catalog, so the entry survived. Soft references are cleared under heap pressure, which Metaspace pressure never causes, so these were permanent in practice. Look through the reference, check the entry's key as well, and stop skipping threads that are not named Gravitino-webserver-*, since catalog.close() itself runs on a ForkJoinPool worker. - HiveClientFactory builds a nested HiveClientClassLoader whose base is the catalog's own loader, and only the catalog loader was ever passed to the cleaner. Clean the nested loader too, so the context ClassLoader that the JDK's pooled process-reaper threads inherit from Hadoop's Shell no longer pins either layer. Claude-Session: https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK
…es of a dropped catalog Widening the coverage to every provider that can be exercised locally turned up four more ways a dropped catalog's ClassLoader stays alive. - DriverManager keeps registered drivers in a static list, and it filters both getDrivers() and deregisterDriver() by the class loader of the calling class. ClassLoaderPool called them from the server's loader, where the catalog's drivers are neither visible nor removable, so every JDBC catalog leaked. Define a small deregisterer inside the catalog's loader and call it there, and drop the pool's version, which could never have worked. - PostgreSQL's driver starts a LazyCleaner thread whose class the catalog defined. A running thread is a GC root, so it pins the loader no matter what its context ClassLoader says; match a thread by the class of the thread and of its runnable as well. - MySQL Connector/J parks an abandoned-connection cleanup executor in a static field, and its thread factory is a lambda the catalog defined. Shut it down through the driver's own uncheckedShutdown(). - Hadoop's cloud connectors install a JCA security provider, such as the shaded OpenSSLProvider in the AWS bundle, into the JVM-wide Security list. Remove the providers the loader installed. Claude-Session: https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK
…pooled catalog tasks Two more pins found while widening the coverage to Oracle and Glue. ResourceBundle caches bundles in a JVM-wide static map behind soft references, so a driver that loads message bundles, such as Oracle's ErrorMessages, leaves its class reachable until heap pressure clears the reference. Metaspace pressure never causes that. clearCache(loader) is the API for it. A task scheduled on a shared executor carries none of the references a thread is matched by today: the AWS SDK's idle-connection reaper is a Runnable on a pool worker, so neither the thread's class, its runnable, nor its context ClassLoader names the catalog. Match on the classes on the thread's stack as well, skipping the calling thread so cleanup never stops itself. Claude-Session: https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK
Code Coverage Report
Files
|
…g the catalog's code
Matching a thread by the classes on its stack was wrong. A request
thread serving an operation on the very catalog being dropped has the
catalog's frames on its stack, so cleanup interrupted it and the request
failed with "Thread was interrupted while waiting for lock". It broke
CatalogHive{2,3}IT#testAlterCatalogProperties and #testListTables, which
alter a catalog while other operations are in flight.
Ownership has to be read from the thread itself, not from what it
happens to be running: its own class, its runnable, or its context
ClassLoader. Those still cover the case the stack rule was added for on
the providers where it was actually confirmed, PostgreSQL's LazyCleaner
among them; it never did fix the AWS idle-connection reaper it was
written for, which needs the Glue catalog to close its client instead
and is tracked in apache#13016.
Claude-Session: https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK
Contributor
Author
|
The Backend Integration Test failure was mine, and it had a clear signature: Cause: the last commit matched a thread against the ClassLoader by the classes on its stack. A request thread serving an operation on the catalog being altered has the catalog's frames on its stack, so cleanup interrupted it mid-request. Altering a catalog releases and re-acquires the pooled ClassLoader, which is why exactly those two tests caught it. Fixed in 2cc3a6b: ownership is read from the thread itself again, its own class, its runnable or its context ClassLoader, never from what it happens to be executing. I re-ran the leak measurements after the revert and Added |
roryqi
approved these changes
Sep 9, 2026
yuqi1129
added a commit
that referenced
this pull request
Sep 9, 2026
…oader of a dropped catalog (#12987) (#13028) **Cherry-pick Information:** - Original commit: ea1d960 - Target branch: `branch-1.3` - Status: Conflicts resolved in c419df5; 296 unit tests passed (Docker tests excluded). Resolution preserves branch-1.3 KerberosClient and removes ClassLoaderPool, which does not exist on the target branch. The original resource cleanup changes and tests are retained. --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
All in
ClassLoaderResourceCleanerUtils, so every provider and every caller of the cleaner benefits:clearThreadLocalMaplooks through ajava.lang.ref.Referencevalue to its referent when deciding whether a thread-local entry belongs to the dying loader, also checks the entry's key, and no longer skips threads that are not namedGravitino-webserver-*.runningWithClassLoadermatches a thread by the class of the thread and of its runnable, not only by its context ClassLoader.removeLoggerContextListeners: removes listeners the loader registered on the shared Log4jLoggerContext.deregisterJdbcDrivers: definesJdbcDriverDeregistererinside the catalog's loader and calls it there, becauseDriverManagerfilters bothgetDrivers()andderegisterDriver()by the caller's ClassLoader.shutdownMysqlConnectionCleanup: calls Connector/J'suncheckedShutdown().removeSecurityProviders: removes JCA providers the loader installed.clearResourceBundleCache:ResourceBundle.clearCache(loader), since bundles are cached JVM-wide behind soft references.HiveClientFactory.close()runs the cleaner against the nestedHiveClientClassLoaderbefore closing it.ClassLoaderPool.deregisterAllDriversis removed: it ran from the server's ClassLoader, where the catalog's drivers are invisible, so it never deregistered anything. The cleaner now covers it.Why are the changes needed?
Dropping or altering a catalog leaked its ClassLoader, so Metaspace grew until the JVM could no longer load new classes and the server degraded into per-feature 500s while already-warm paths kept returning 200. Since an
alterrebuilds the ClassLoader, the loss accumulated: five alters of a Hive catalog cost ~48 MB that was never returned.Every retention path was traced from a heap dump back to a GC root. They are unrelated to each other, which is why the fix has several parts:
LoggerContextSoftReferencein aThreadLocal(JacksonBufferRecycler)HiveClientClassLoadernever cleanedDriverManager.registeredDriversLazyCleanerthreadAbandonedConnectionCleanupThreadexecutorOpenSSLProviderfrom the AWS bundle)ResourceBundlecache (soft)ErrorMessages, any localized driverSoft references deserve a note: they are cleared under heap pressure, and Metaspace pressure never triggers that, so on a server with a roomy heap and a small
MaxMetaspaceSizea soft-referenced loader is permanent in practice.Fix: #12986
Does this PR introduce any user-facing change?
No new configuration or API. Dropped catalogs release their Metaspace, so a long-running server no longer grows without bound.
How was this patch tested?
Unit tests: 7 new cases in
TestClassLoaderResourceCleanerUtils(looking through a reference, cleared references, clearing a soft-referenced thread-local, leaving unrelated entries alone, matching a thread by its runnable, leaving unrelated security providers alone).:catalogs:catalog-common,:catalogs:hive-metastore-common,:catalogs:catalog-hive,:catalogs:catalog-fileset,:catalogs:catalog-jdbc-commonand the:coreClassLoader tests pass.End-to-end on a packaged server with
-Xms1024m -Xmx1024m -XX:MaxMetaspaceSize=512m, one catalog at a time: create, exercise (schema plus a table, fileset, topic or model version), drop, force a full GC, then count loaders withjcmd VM.classloader_statsand read Metaspace withjcmd GC.heap_info. Backends were a real Hive metastore, MySQL, PostgreSQL, Kafka and MinIO in containers.modelfileset(file://)fileset(s3a, MinIO)fileset(gs://)fileset(abfss://)jdbc-mysqljdbc-postgresqlkafkahivelakehouse-paimonlakehouse-iceberg(jdbc backend)Repeated churn, the case that exhausts Metaspace in practice — five alters of a Hive catalog: before, 7 loaders and 70.6 → 119.3 MB that never came back; after, back to baseline at +0.8 MB.
glueis the one provider still not released, verified against LocalStack. Its root is different in kind: the AWS SDK'sIdleConnectionReaperis a singleton per ClassLoader that only stops once every connection manager is deregistered, and it ignores interrupts, so some AWS client the catalog builds is not being closed. That is a client-lifecycle bug in the catalog rather than a cleanup gap, and papering over it by reflecting into SDK internals seemed worse than reporting it; I will file it separately.Not covered locally, for lack of a backend:
jdbc-doris,jdbc-starrocks,lakehouse-hudi,lakehouse-generic. Doris and StarRocks use the MySQL driver, so the DriverManager and Connector/J fixes apply to them unchanged.https://claude.ai/code/session_013xVSteM2ZUjXRHFbHtayVK