mchades commented on code in PR #8252:
URL: https://github.com/apache/gravitino/pull/8252#discussion_r2302626226
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java:
##########
@@ -271,6 +280,211 @@ public void close() throws IOException {
catalogUserContext.close();
UserContext.cleanAllUserContext();
+
+ boolean testEnv = System.getenv("GRAVITINO_TEST") != null;
+ if (testEnv) {
+ // In test environment, we do not need to clean up class loader related
stuff
Review Comment:
The comment should explain "why", and the real reason is that you mentioned
in the previous comment that "there is only one class loader in the test
environment."
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/SecureFilesetCatalogOperations.java:
##########
@@ -271,6 +280,211 @@ public void close() throws IOException {
catalogUserContext.close();
UserContext.cleanAllUserContext();
+
+ boolean testEnv = System.getenv("GRAVITINO_TEST") != null;
+ if (testEnv) {
+ // In test environment, we do not need to clean up class loader related
stuff
+ return;
+ }
+
+ try {
+ closeStatsDataClearerInFileSystem();
+
+ stopThreadsAndClearThreadLocal();
+
+ // Release the LogFactory for the FilesetCatalogOperations class loader
+ unregisterLogFactory();
+
+ closeResourceInAWS();
+
+ closeResourceInGCP();
+
+ closeResourceInAzure();
+
+ clearShutdownHooks();
+ } catch (Exception e) {
+ LOG.warn("Failed to clear resources(Thread, ThreadLocal variants) in the
class loader", e);
+ }
+ }
+
+ private static void stopThreadsAndClearThreadLocal() {
+ // Clear all thread references to the ClosableHiveCatalog class loader and
clear thread local
+ Thread[] threads = getAllThreads();
+ for (Thread thread : threads) {
+ // Clear thread local map for webserver threads in the current class
loader. Why do we need
+ // this? Because the webserver threads are long-lived threads and will
holds may thread
+ // local references to the current class loader. However, they can't be
cleared as
+ // `Catalog.close` is called in the thread pool located in the Caffeine
cache, which is not
+ // in the webserver threads. So we need to manually clear the thread
local map for webserver
+ // threads.
+ clearThreadLocalMap(thread);
+
+ // Close all threads that are using the FilesetCatalogOperations class
loader
+ if (runningWithCurrentClassLoader(thread)) {
+ LOG.info("Interrupting thread: {}", thread.getName());
+ thread.setContextClassLoader(null);
+ thread.interrupt();
+ try {
+ thread.join(5000);
Review Comment:
Does this conflict with the `Thread.sleep(500);` in the CatalogManager?
--
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]
