danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3932590211
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##########
@@ -318,12 +336,85 @@ public void close() {
if (metaStoreClient != null) {
Hive.closeCurrent();
}
- if (hiveDriver != null) {
- try {
- hiveDriver.close();
- } finally {
- destroyQuietly(hiveDriver);
+ closeDriverAndSession();
Review Comment:
[P1] Close the SessionState before clearing the Hive thread local
At this point Hive.closeCurrent() has already removed the executor Hive
instance. In Hive 2.3.10, SessionState.close() (called here) invokes
unCacheDataNucleusClassLoaders(), which calls Hive.get(sessionConf).getMSC()
and finally only removes that new Hive from the thread local. With no current
Hive, this creates a fresh metastore client and then drops its only reference
without closing it; the outer HoodieHiveSyncClient.close() only closes the
original client. This can leak one HMS connection per sync. Please run
closeDriverAndSession() before Hive.closeCurrent() (with the latter in a
finally so it still runs on teardown failure), allowing session cleanup to
reuse the original Hive/client.
--
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]
