This is an automated email from the ASF dual-hosted git repository.
danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new e0fe585bbd4c fix(hive-sync): call Driver.destroy() so HiveQL sync
stops leaking Drivers into ShutdownHookManager (#19718)
e0fe585bbd4c is described below
commit e0fe585bbd4c5eab8a6bdbb93972e19fc454956d
Author: Shihuan Liu <[email protected]>
AuthorDate: Mon Aug 24 21:05:42 2026 -0700
fix(hive-sync): call Driver.destroy() so HiveQL sync stops leaking Drivers
into ShutdownHookManager (#19718)
* fix(hive-sync): call Driver.destroy() so HiveQL sync stops leaking
Drivers into ShutdownHookManager
* Make the Driver hook removal survive a failing close()
Driver.close() is not documented to be exception free, and its internal
release steps only swallow exceptions individually, so a close() that does
throw would skip destroy() and leave exactly the hook we are trying to
remove. Attempt destroy() independently of close() at all three sites.
destroy() reports rather than rethrows its own failure: it ends up in
ShutdownHookManager.removeShutdownHook, which throws once JVM shutdown has
begun, and at that point the hook set no longer matters. On the constructor
path this also stops a failing teardown from masking the construction error.
---
.../apache/hudi/hive/ddl/HiveQueryDDLExecutor.java | 29 ++++++++++++-
.../org/apache/hudi/hive/util/HiveDriverPool.java | 10 +++++
.../apache/hudi/hive/util/TestHiveDriverPool.java | 50 ++++++++++++++++++++++
3 files changed, 87 insertions(+), 2 deletions(-)
diff --git
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java
index 5447e1fd922f..2002420a4d60 100644
---
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java
+++
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java
@@ -92,7 +92,12 @@ public class HiveQueryDDLExecutor extends
QueryBasedDDLExecutor {
}
}
if (this.hiveDriver != null) {
- this.hiveDriver.close();
+ try {
+ this.hiveDriver.close();
+ } catch (Exception driverCloseException) {
+ log.error("Error while closing Hive Driver", driverCloseException);
+ }
+ destroyQuietly(this.hiveDriver);
}
// driverPool (if present) was already constructed by the caller before
this
// ctor ran; since we're about to throw, no one else will call close()
on it.
@@ -314,7 +319,27 @@ public class HiveQueryDDLExecutor extends
QueryBasedDDLExecutor {
Hive.closeCurrent();
}
if (hiveDriver != null) {
- hiveDriver.close();
+ try {
+ hiveDriver.close();
+ } finally {
+ destroyQuietly(hiveDriver);
+ }
+ }
+ }
+
+ /**
+ * Removes the shutdown hook that {@link Driver#compile} registered. A fresh
HiveSyncTool, and
+ * therefore a fresh Driver, is built per sync, and close() leaves that hook
in place, so without
+ * this every sync permanently adds a Driver to the static {@code
ShutdownHookManager}. Runs even
+ * when close() failed, and reports rather than rethrows its own failure:
destroy() ends up in
+ * {@code ShutdownHookManager.removeShutdownHook}, which refuses to run once
JVM shutdown has
+ * begun, and by then the hook set no longer matters.
+ */
+ private static void destroyQuietly(Driver driver) {
+ try {
+ driver.destroy();
+ } catch (Exception e) {
+ log.warn("Error while destroying Hive Driver", e);
}
}
}
diff --git
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveDriverPool.java
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveDriverPool.java
index b9f4db106ac1..e73eee19cf5e 100644
---
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveDriverPool.java
+++
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveDriverPool.java
@@ -210,6 +210,16 @@ public class HiveDriverPool implements AutoCloseable {
} catch (Exception e) {
LOG.warn("Error closing pooled Driver", e);
}
+ // close() releases the current query's resources but leaves the
shutdown hook
+ // Driver.compile() registered; only destroy() removes it. Without
this the
+ // static ShutdownHookManager keeps every pooled Driver -- and the
Table and
+ // FieldSchema objects of its last query -- alive for the life of
the JVM. This
+ // gets its own try so a failing close() cannot skip the hook
removal.
+ try {
+ worker.driver.destroy();
+ } catch (Exception e) {
+ LOG.warn("Error destroying pooled Driver", e);
+ }
}
if (worker.sessionState != null) {
try {
diff --git
a/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/util/TestHiveDriverPool.java
b/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/util/TestHiveDriverPool.java
index 0f5df5a54399..efb99c5d5bbe 100644
---
a/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/util/TestHiveDriverPool.java
+++
b/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/util/TestHiveDriverPool.java
@@ -48,6 +48,7 @@ import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
/**
* Unit tests for {@link HiveDriverPool} that exercise bootstrap, dispatch,
error
@@ -185,6 +186,55 @@ class TestHiveDriverPool {
() -> pool.dispatchAll(Arrays.asList("anything")));
}
+ /**
+ * Driver.compile() registers a shutdown hook that Driver.close() does not
remove -- only
+ * destroy() does. Closing a pooled Driver without destroying it therefore
leaks it, and
+ * everything its last query referenced, into the static ShutdownHookManager
for the life
+ * of the JVM. A long-running sync loop builds a pool per sync, so this
grows without bound.
+ */
+ @Test
+ void closeDestroysEachPooledDriver() throws Exception {
+ HiveSyncConfig config = configWithEmptyHiveConf();
+ List<Driver> drivers = Collections.synchronizedList(new ArrayList<>());
+ HiveDriverPool.DriverFactory factory = (db) -> {
+ Driver d = mock(Driver.class);
+ drivers.add(d);
+ return d;
+ };
+ HiveDriverPool pool = new HiveDriverPool(config, 3, factory);
+ pool.close();
+
+ assertEquals(3, drivers.size());
+ for (Driver d : drivers) {
+ verify(d, times(1)).close();
+ verify(d, times(1)).destroy();
+ }
+ }
+
+ /**
+ * The hook removal is the whole point of destroy(), so a Driver whose
close() blows up must
+ * still be destroyed -- otherwise the failure that made teardown
interesting is also the one
+ * that leaks the Driver.
+ */
+ @Test
+ void closeDestroysPooledDriverEvenWhenCloseThrows() throws Exception {
+ HiveSyncConfig config = configWithEmptyHiveConf();
+ List<Driver> drivers = Collections.synchronizedList(new ArrayList<>());
+ HiveDriverPool.DriverFactory factory = (db) -> {
+ Driver d = mock(Driver.class);
+ when(d.close()).thenThrow(new RuntimeException("close failed"));
+ drivers.add(d);
+ return d;
+ };
+ HiveDriverPool pool = new HiveDriverPool(config, 2, factory);
+ pool.close();
+
+ assertEquals(2, drivers.size());
+ for (Driver d : drivers) {
+ verify(d, times(1)).destroy();
+ }
+ }
+
@Test
void invalidSizeRejected() {
HiveSyncConfig config = configWithEmptyHiveConf();
