From 67670a1d93814bc08e7bd497c77879582b7ed022 Mon Sep 17 00:00:00 2001
From: joeyutong <joeyutong@gmail.com>
Date: Mon, 5 Jan 2026 10:57:37 +0800
Subject: [PATCH 1/3] [#9581] fix(hive): Perform resource cleanup in
 HiveClientPool close

---
 .../hive-metastore-common/build.gradle.kts    |  1 +
 .../apache/gravitino/hive/HiveClientPool.java |  6 +-
 .../gravitino/hive/TestHiveClientPool.java    | 97 +++++++++++++++++++
 3 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java

diff --git a/catalogs/hive-metastore-common/build.gradle.kts b/catalogs/hive-metastore-common/build.gradle.kts
index 406b0fd31cf..de365651891 100644
--- a/catalogs/hive-metastore-common/build.gradle.kts
+++ b/catalogs/hive-metastore-common/build.gradle.kts
@@ -120,6 +120,7 @@ dependencies {
     exclude("org.slf4j")
   }
   testImplementation(libs.junit.jupiter.api)
+  testImplementation(libs.mockito.core)
   testImplementation(libs.woodstox.core)
   testImplementation(libs.testcontainers)
   testImplementation(project(":integration-test-common", "testArtifacts"))
diff --git a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
index 7fa0098a34d..fb958db6623 100644
--- a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
+++ b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
@@ -41,7 +41,7 @@ public class HiveClientPool extends ClientPoolImpl<HiveClient, GravitinoRuntimeE
    * @param properties The configuration used to initialize the Hive Metastore clients.
    */
   public HiveClientPool(String name, int poolSize, Properties properties) {
-    // Do not allow retry by default as we rely on RetryingHiveClient
+    // Do not allow retry by default as we rely on RetryingMetaStoreClient
     super(poolSize, GravitinoRuntimeException.class, false);
     this.clientFactory = new HiveClientFactory(properties, name);
   }
@@ -56,12 +56,15 @@ protected HiveClient newClient() {
     }
   }
 
+  // Provides a no-op reconnect implementation because we rely on the RetryingMetaStoreClient
+  // to do the actual reconnect logic.
   @Override
   protected HiveClient reconnect(HiveClient client) {
     LOG.warn("Reconnecting to Hive Metastore");
     return client;
   }
 
+  // Returns false because we do not need pool-level reconnections.
   @Override
   protected boolean isConnectionException(Exception e) {
     return false;
@@ -70,5 +73,6 @@ protected boolean isConnectionException(Exception e) {
   @Override
   protected void close(HiveClient client) {
     LOG.info("Closing Hive Metastore client");
+    client.close();
   }
 }
diff --git a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
new file mode 100644
index 00000000000..21c8016d438
--- /dev/null
+++ b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.gravitino.hive;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.Lists;
+import java.util.List;
+import java.util.Properties;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.hive.client.HiveClient;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+// Referred from Apache Iceberg's TestHiveClientPool implementation
+// hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
+public class TestHiveClientPool {
+
+  HiveClientPool clients;
+
+  @BeforeEach
+  public void before() {
+    HiveClientPool clientPool = new HiveClientPool("hive", 2, new Properties());
+    clients = Mockito.spy(clientPool);
+  }
+
+  @AfterEach
+  public void after() {
+    clients.close();
+    clients = null;
+  }
+
+  @Test
+  public void testNewClientFailure() {
+    Mockito.doThrow(new RuntimeException("Connection exception")).when(clients).newClient();
+    RuntimeException ex = assertThrows(RuntimeException.class, () -> clients.run(Object::toString));
+    assertEquals("Connection exception", ex.getMessage());
+  }
+
+  @Test
+  public void testReconnect() {
+    HiveClient hiveClient = newClient();
+
+    String metaMessage = "Got exception: org.apache.thrift.transport.TTransportException";
+    Mockito.doThrow(new GravitinoRuntimeException(metaMessage))
+        .when(hiveClient)
+        .getAllDatabases("");
+
+    GravitinoRuntimeException ex =
+        assertThrows(
+            GravitinoRuntimeException.class,
+            () -> clients.run(client -> client.getAllDatabases("")));
+    assertEquals("Got exception: org.apache.thrift.transport.TTransportException", ex.getMessage());
+    // Verify that the method is never called.
+    Mockito.verify(clients, Mockito.never()).reconnect(hiveClient);
+  }
+
+  @Test
+  public void testClose() throws Exception {
+    HiveClient hiveClient = newClient();
+
+    List<String> databases = Lists.newArrayList("db1", "db2");
+    Mockito.doReturn(databases).when(hiveClient).getAllDatabases("");
+    assertEquals(clients.run(client -> client.getAllDatabases("")), databases);
+
+    clients.close();
+    assertTrue(clients.isClosed());
+    Mockito.verify(hiveClient).close();
+  }
+
+  private HiveClient newClient() {
+    HiveClient hiveClient = Mockito.mock(HiveClient.class);
+    Mockito.doReturn(hiveClient).when(clients).newClient();
+    return hiveClient;
+  }
+}

From 91751422d67ccacacc3c17d144ccbc7260bc7f8e Mon Sep 17 00:00:00 2001
From: joeyutong <joeyutong@gmail.com>
Date: Mon, 5 Jan 2026 10:57:37 +0800
Subject: [PATCH 2/3] [#9581] fix(hive): Perform resource cleanup in
 HiveClientPool close

---
 .../java/org/apache/gravitino/hive/HiveClientPool.java    | 8 +++++---
 .../org/apache/gravitino/hive/TestHiveClientPool.java     | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
index fb958db6623..d2058524e78 100644
--- a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
+++ b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
@@ -56,15 +56,17 @@ protected HiveClient newClient() {
     }
   }
 
-  // Provides a no-op reconnect implementation because we rely on the RetryingMetaStoreClient
-  // to do the actual reconnect logic.
+  /**
+   * Provides a no-op reconnect implementation. Reconnect logic is handled by
+   * RetryingMetaStoreClient.
+   */
   @Override
   protected HiveClient reconnect(HiveClient client) {
     LOG.warn("Reconnecting to Hive Metastore");
     return client;
   }
 
-  // Returns false because we do not need pool-level reconnections.
+  /** Returns false by design. Pool-level reconnection is not required. */
   @Override
   protected boolean isConnectionException(Exception e) {
     return false;
diff --git a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
index 21c8016d438..766d50b205c 100644
--- a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
+++ b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
@@ -37,7 +37,7 @@
 // hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
 public class TestHiveClientPool {
 
-  HiveClientPool clients;
+  private HiveClientPool clients;
 
   @BeforeEach
   public void before() {

From 597b6d17787ec2df6a082858dbd3d54bf35aaaaf Mon Sep 17 00:00:00 2001
From: joeyutong <joeyutong@gmail.com>
Date: Mon, 5 Jan 2026 10:57:37 +0800
Subject: [PATCH 3/3] [#9581] fix(hive): Perform resource cleanup in
 HiveClientPool close

---
 .../java/org/apache/gravitino/hive/HiveClientPool.java     | 7 ++-----
 .../java/org/apache/gravitino/hive/TestHiveClientPool.java | 7 +++++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
index d2058524e78..e4d78e58c76 100644
--- a/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
+++ b/catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/HiveClientPool.java
@@ -56,19 +56,16 @@ protected HiveClient newClient() {
     }
   }
 
-  /**
-   * Provides a no-op reconnect implementation. Reconnect logic is handled by
-   * RetryingMetaStoreClient.
-   */
   @Override
   protected HiveClient reconnect(HiveClient client) {
+    // No-op reconnect: RetryingMetaStoreClient handles reconnect logic.
     LOG.warn("Reconnecting to Hive Metastore");
     return client;
   }
 
-  /** Returns false by design. Pool-level reconnection is not required. */
   @Override
   protected boolean isConnectionException(Exception e) {
+    // Pool-level reconnection is not required by design.
     return false;
   }
 
diff --git a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
index 766d50b205c..83f397606d4 100644
--- a/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
+++ b/catalogs/hive-metastore-common/src/test/java/org/apache/gravitino/hive/TestHiveClientPool.java
@@ -33,8 +33,11 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
-// Referred from Apache Iceberg's TestHiveClientPool implementation
-// hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
+/**
+ * Referenced from Apache Iceberg's {@code TestHiveClientPool} implementation.
+ *
+ * <p>Source: hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
+ */
 public class TestHiveClientPool {
 
   private HiveClientPool clients;