(gravitino) branch main updated: [#8999]Improvement(iceberg-common): Add closing of the clientPool in ClosableHiveCatalog.close() (#9000)

[yuqi4733]

This is an automated email from the ASF dual-hosted git repository.

yuqi4733 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git


The following commit(s) were added to refs/heads/main by this push:
     new 73e9003b91 [#8999]Improvement(iceberg-common): Add closing of the 
clientPool in ClosableHiveCatalog.close() (#9000)
73e9003b91 is described below

commit 73e9003b91df13b1b4e80ffd543fce6a5d5d455a
Author: Xiaojian Sun <sunxiaojian...@163.com>
AuthorDate: Wed Nov 5 14:43:43 2025 +0800

    [#8999]Improvement(iceberg-common): Add closing of the clientPool in 
ClosableHiveCatalog.close() (#9000)
    
    ### What changes were proposed in this pull request?
    
    Add closing of the clientPool in ClosableHiveCatalog.close()
    
    ### Why are the changes needed?
    
    Fix: #8999
    
    ### Does this PR introduce _any_ user-facing change?
    
    N/A
    
    ### How was this patch tested?
    
    N/A
---
 .../iceberg/common/ClosableHiveCatalog.java        | 33 ++++++++++++++++++++++
 .../iceberg/common/ops/IcebergCatalogWrapper.java  |  6 ++--
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git 
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ClosableHiveCatalog.java
 
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ClosableHiveCatalog.java
index 6160d38a7d..88db666d83 100644
--- 
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ClosableHiveCatalog.java
+++ 
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ClosableHiveCatalog.java
@@ -22,7 +22,9 @@ package org.apache.gravitino.iceberg.common;
 import com.google.common.collect.Lists;
 import java.io.Closeable;
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.util.List;
+import org.apache.gravitino.iceberg.common.utils.IcebergHiveCachedClientPool;
 import org.apache.iceberg.hive.HiveCatalog;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,6 +51,11 @@ public class ClosableHiveCatalog extends HiveCatalog 
implements Closeable {
   public void close() throws IOException {
     // Do clean up work here. We need a mechanism to close the HiveCatalog; 
however, HiveCatalog
     // doesn't implement the Closeable interface.
+
+    // First, close the internal HiveCatalog client pool to prevent resource 
leaks
+    closeInternalClientPool();
+
+    // Then close any additional resources added via addResource()
     resources.forEach(
         resource -> {
           try {
@@ -60,4 +67,30 @@ public class ClosableHiveCatalog extends HiveCatalog 
implements Closeable {
           }
         });
   }
+
+  /**
+   * Close the internal HiveCatalog client pool using reflection. This is 
necessary because
+   * HiveCatalog doesn't provide a public API to close its client pool. We 
need to avoid closing
+   * IcebergHiveCachedClientPool twice (once here and once in resources list).
+   */
+  private void closeInternalClientPool() {
+    try {
+      Field clientsField = HiveCatalog.class.getDeclaredField("clients");
+      clientsField.setAccessible(true);
+      Object clientPool = clientsField.get(this);
+
+      if (clientPool != null && clientPool instanceof AutoCloseable) {
+        // Only close if it's NOT IcebergHiveCachedClientPool
+        if (!(clientPool instanceof IcebergHiveCachedClientPool)) {
+          ((AutoCloseable) clientPool).close();
+          LOGGER.info(
+              "Closed HiveCatalog internal client pool: {}", 
clientPool.getClass().getSimpleName());
+        }
+      }
+    } catch (NoSuchFieldException e) {
+      LOGGER.warn("Could not find 'clients' field in HiveCatalog, skipping 
cleanup", e);
+    } catch (Exception e) {
+      LOGGER.warn("Failed to close HiveCatalog internal client pool", e);
+    }
+  }
 }
diff --git 
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
 
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
index e8ad09d4bf..bc9f7c0c09 100644
--- 
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
+++ 
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
@@ -280,7 +280,8 @@ public class IcebergCatalogWrapper implements AutoCloseable 
{
   @Override
   public void close() throws Exception {
     if (catalog instanceof AutoCloseable) {
-      // JdbcCatalog and WrappedHiveCatalog need close.
+      // JdbcCatalog and ClosableHiveCatalog implement AutoCloseable and will 
handle their own
+      // cleanup
       ((AutoCloseable) catalog).close();
     }
     metadataCache.close();
@@ -296,9 +297,6 @@ public class IcebergCatalogWrapper implements AutoCloseable 
{
       closeMySQLCatalogResource();
     } else if (catalogUri != null && catalogUri.contains("postgresql")) {
       closePostgreSQLCatalogResource();
-    } else if (catalogBackend.equals(IcebergCatalogBackend.HIVE)) {
-      // TODO(yuqi) add close for other catalog types such Hive catalog, for 
more, please refer to
-      // 
https://github.com/apache/gravitino/pull/2548/commits/ab876b69b7e094bbd8c174d48a2365a18ed5176d
     }
   }