Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 merged PR #19812: URL: https://github.com/apache/hudi/pull/19812 -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5547808140 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 117a0d34975d83007f188039c34be8ffa31d9e1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16834) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5548156645 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 117a0d34975d83007f188039c34be8ffa31d9e1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16834) * 401ae3defa0a29f0c199d78327daeb05e7050595 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16836) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5547858425 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 117a0d34975d83007f188039c34be8ffa31d9e1b Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16834) * 401ae3defa0a29f0c199d78327daeb05e7050595 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3938674169
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -77,27 +77,28 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.metaStoreClient = metaStoreClient;
this.driverPool = driverPool;
this.metaStoreClientPool = metaStoreClientPool;
+// SessionState.start() binds the session it starts to this thread,
displacing the caller's.
+// Statements and the teardown bind ours themselves, so the thread is
handed back once the
+// Driver is built -- its constructor is what reads SessionState.get().
+SessionState previousSession = SessionState.get();
+ClassLoader previousLoader =
Thread.currentThread().getContextClassLoader();
try {
- this.sessionState = new SessionState(config.getHiveConf(),
+ // Its own conf copy, as HiveDriverPool's workers get: a session stamps
hive.session.id and a
+ // UDFClassLoader onto the conf it is handed, and close() deletes the
directories that id
+ // names and closes that loader. config's HiveConf outlives us and both
pools copy it.
+ HiveConf sessionConf = new HiveConf(config.getHiveConf());
+ // An inherited ID would make close() delete the caller's session
directories.
+ sessionConf.setVar(HiveConf.ConfVars.HIVESESSIONID, "");
+ this.sessionState = new SessionState(sessionConf,
UserGroupInformation.getCurrentUser().getShortUserName());
Review Comment:
[P2] Isolate the downloaded-resource directory as well
The fresh session ID isolates the scratch paths, but
`hive.downloaded.resources.dir` is also copied unchanged. When that setting is
a fixed path (for example `/tmp/app-hive-resources`, without
`${hive.session.id}`), the executor and caller still share it. Hive 2.3.10's
`SessionState.close()` recursively deletes the configured directory itself; it
does not append the session ID. The new teardown therefore removes the live
caller's downloaded JARs/files, which can break subsequent queries that use
those resource paths.
Reproduced at this head with two real sessions: create a resource file in
the caller's configured directory, construct the executor from that caller's
conf, and close it. The caller is restored but its file is deleted. A control
using a `${hive.session.id}_resources` path passes.
Please give the executor a private downloaded-resource directory in its
copied conf before creating the session, and extend the ownership regression
test to assert that caller resources survive as well as the local/HDFS scratch
files.
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3938621649
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -77,27 +77,26 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.metaStoreClient = metaStoreClient;
this.driverPool = driverPool;
this.metaStoreClientPool = metaStoreClientPool;
+// SessionState.start() binds the session it starts to this thread,
displacing the caller's.
+// Statements and the teardown bind ours themselves, so the thread is
handed back once the
+// Driver is built -- its constructor is what reads SessionState.get().
+SessionState previousSession = SessionState.get();
+ClassLoader previousLoader =
Thread.currentThread().getContextClassLoader();
try {
- this.sessionState = new SessionState(config.getHiveConf(),
+ // Its own conf copy, as HiveDriverPool's workers get: a session stamps
hive.session.id and a
+ // UDFClassLoader onto the conf it is handed, and close() deletes the
directories that id
+ // names and closes that loader. config's HiveConf outlives us and both
pools copy it.
+ HiveConf sessionConf = new HiveConf(config.getHiveConf());
Review Comment:
[P1] Give the owned session a fresh session ID
Copying `HiveConf` preserves an existing `hive.session.id`, and Hive's
`SessionState` constructor generates an ID only when that value is empty. If
the supplied configuration comes from a live Hive session, this executor
therefore shares that session's local and HDFS scratch directories. The newly
added session teardown deletes those directories, including the caller's
scratch files and temporary-table storage, even though the caller's session is
restored afterward.
I reproduced this with a real caller `SessionState`: build `HiveSyncConfig`
from its conf, create the executor, and close it. A marker file in the
still-live caller's local session directory is deleted. The 11 existing
lifecycle tests pass but do not cover an inherited session ID.
Please clear the ID on the private copy before constructing `SessionState`,
allowing Hive to generate a fresh one, and add a regression test for the
inherited-ID case:
```suggestion
HiveConf sessionConf = new HiveConf(config.getHiveConf());
sessionConf.setVar(HiveConf.ConfVars.HIVESESSIONID, "");
```
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5547143752 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 49a69f2ece94fb158a11203c3970b81c85425fc8 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16833) * 117a0d34975d83007f188039c34be8ffa31d9e1b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16834) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5547003184 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 49a69f2ece94fb158a11203c3970b81c85425fc8 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16833) * 117a0d34975d83007f188039c34be8ffa31d9e1b UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3938162943
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -77,27 +77,32 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.metaStoreClient = metaStoreClient;
this.driverPool = driverPool;
this.metaStoreClientPool = metaStoreClientPool;
+// SessionState.start() attaches the session it starts to this thread,
displacing whatever the
+// caller had there -- another executor's session, or that of an
application embedding this
+// sync. Ours is not the thread's to keep: every statement and the
teardown bind it
+// explicitly, so give the thread back once the Driver, whose constructor
reads
+// SessionState.get(), has been built.
+SessionState previousSession = SessionState.get();
+ClassLoader previousLoader =
Thread.currentThread().getContextClassLoader();
try {
- this.sessionState = new SessionState(config.getHiveConf(),
+ // The session gets a conf of its own because it does not just read one:
its constructor
Review Comment:
Simplified
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-agent commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3937709418
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -77,27 +77,32 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.metaStoreClient = metaStoreClient;
this.driverPool = driverPool;
this.metaStoreClientPool = metaStoreClientPool;
+// SessionState.start() attaches the session it starts to this thread,
displacing whatever the
+// caller had there -- another executor's session, or that of an
application embedding this
+// sync. Ours is not the thread's to keep: every statement and the
teardown bind it
+// explicitly, so give the thread back once the Driver, whose constructor
reads
+// SessionState.get(), has been built.
+SessionState previousSession = SessionState.get();
+ClassLoader previousLoader =
Thread.currentThread().getContextClassLoader();
try {
- this.sessionState = new SessionState(config.getHiveConf(),
+ // The session gets a conf of its own because it does not just read one:
its constructor
Review Comment:
🤖 nit: these inline comments are quite dense (7+ lines of narrative prose
here, similar blocks throughout). Could you tighten them to the key invariant —
e.g. "session needs its own HiveConf copy so close() doesn't delete scratch
dirs / close the loader shared with config's HiveConf" — and lean on the method
Javadocs for the rest? It'd be easier for a future reader to skim.
⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5545674403 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 49a69f2ece94fb158a11203c3970b81c85425fc8 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16833) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5544587927 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * d55315bb077f722ea5310eb9871d32dfe79d8d0e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16820) * 49a69f2ece94fb158a11203c3970b81c85425fc8 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16833) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5544139561 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * d55315bb077f722ea5310eb9871d32dfe79d8d0e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16820) * 49a69f2ece94fb158a11203c3970b81c85425fc8 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3936403968
##
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:
Done, thanks!
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5534906197 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * d55315bb077f722ea5310eb9871d32dfe79d8d0e Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16820) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5534467540 ## CI report: * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16819) * d55315bb077f722ea5310eb9871d32dfe79d8d0e Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16820) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5534149455 ## CI report: * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16819) * d55315bb077f722ea5310eb9871d32dfe79d8d0e Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16820) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533874739 ## CI report: * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16819) * d55315bb077f722ea5310eb9871d32dfe79d8d0e UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3929782775
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -183,6 +180,12 @@ private List
updateHiveSQLs(List sqls) {
}
} catch (Exception e) {
throw new HoodieHiveSyncException("Failed in executing SQL", e);
+} finally {
+ if (previousSession != null) {
+SessionState.setCurrentSessionState(previousSession);
+ } else {
+SessionState.detachSession();
Review Comment:
Done
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -84,20 +83,10 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.sessionState.setCurrentDatabase(databaseName);
this.hiveDriver = new
org.apache.hadoop.hive.ql.Driver(config.getHiveConf());
} catch (Exception e) {
- if (sessionState != null) {
-try {
- this.sessionState.close();
-} catch (IOException ioException) {
- log.error("Error while closing SessionState", ioException);
-}
- }
- if (this.hiveDriver != null) {
-try {
- this.hiveDriver.close();
-} catch (Exception driverCloseException) {
- log.error("Error while closing Hive Driver", driverCloseException);
-}
-destroyQuietly(this.hiveDriver);
+ try {
+closeDriverAndSession();
Review Comment:
Done
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533784590 ## CI report: * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16819) * d55315bb077f722ea5310eb9871d32dfe79d8d0e UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533736230 ## CI report: * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16819) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533690008 ## CI report: * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN * 81d303d9a7d128d43e2811054bab536fea199d54 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533555478 ## CI report: * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533361613 ## CI report: * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) * 04a3797989eebf8c9c15bf6af9d35ddd6036b00d UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533167100 ## CI report: * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16818) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5533119309 ## CI report: * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) * a369ba706b81a5c75918c93884f67a2582845fdd UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5530243352 ## CI report: * a106b790e40138c9d847e09790d24e284a68d678 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16783) * b07209cb3c847a6f054d098ff2049e49e0e5aaad UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5532920046 ## CI report: * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5532634212 ## CI report: * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16817) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5532513524 ## CI report: * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) * 0d6a3957b458ffe1e3ad7fa8c4756825999161a4 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5532000508 ## CI report: * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) * 4ca102a68fb19f188b38d3a46d7f498fa9150668 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16815) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5531929691 ## CI report: * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) * 4ca102a68fb19f188b38d3a46d7f498fa9150668 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5531440476 ## CI report: * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5530435571 ## CI report: * a106b790e40138c9d847e09790d24e284a68d678 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16783) * b07209cb3c847a6f054d098ff2049e49e0e5aaad Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16811) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3920216225
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -183,6 +180,12 @@ private List
updateHiveSQLs(List sqls) {
}
} catch (Exception e) {
throw new HoodieHiveSyncException("Failed in executing SQL", e);
+} finally {
+ if (previousSession != null) {
+SessionState.setCurrentSessionState(previousSession);
+ } else {
+SessionState.detachSession();
Review Comment:
[P2] Restore the caller context class loader too
Hive `SessionState.setCurrentSessionState` changes
`Thread.currentThread().getContextClassLoader()` to the session conf loader,
while `detachSession()` only removes the Hive thread-local. Thus, when
`previousSession` is null, this finally block leaves the caller thread using
this executor class loader; `closeDriverAndSession` has the same problem and
can leave the thread pointing at a loader that `SessionState.close()` just
closed. That both leaks the session loader and can break later class loading on
long-running worker threads. Please capture the original context class loader
before binding and restore it in the finally path, including the null-session
case, with an assertion in the lifecycle tests.
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3920217136
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -84,20 +83,10 @@ public HiveQueryDDLExecutor(HiveSyncConfig config,
IMetaStoreClient metaStoreCli
this.sessionState.setCurrentDatabase(databaseName);
this.hiveDriver = new
org.apache.hadoop.hive.ql.Driver(config.getHiveConf());
} catch (Exception e) {
- if (sessionState != null) {
-try {
- this.sessionState.close();
-} catch (IOException ioException) {
- log.error("Error while closing SessionState", ioException);
-}
- }
- if (this.hiveDriver != null) {
-try {
- this.hiveDriver.close();
-} catch (Exception driverCloseException) {
- log.error("Error while closing Hive Driver", driverCloseException);
-}
-destroyQuietly(this.hiveDriver);
+ try {
+closeDriverAndSession();
Review Comment:
[P1] Preserve the session displaced by construction
`SessionState.start(this.sessionState)` overwrites any session already
attached to the caller thread, but the constructor never saves or restores that
session. In the real lifecycle, `updateHiveSQLs` therefore observes this
executor session as `previousSession` and restores it, and this teardown
observes the same session, closes/detaches it, and leaves the caller with no
session; the original embedding session is never recovered. The new tests miss
this because `executorWith` bypasses the constructor and manually attaches
`otherSession` afterward. Now that each SQL/teardown operation explicitly binds
the owned session, could the constructor capture the pre-existing session
before `start`, restore it after initialization and on failure, and add a
constructor-path regression test?
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5517412284 ## CI report: * a106b790e40138c9d847e09790d24e284a68d678 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16783) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5516234114 ## CI report: * 031918f2aaa0e81ff10a2270f7e0bf775549bfea Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16782) * a106b790e40138c9d847e09790d24e284a68d678 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16783) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5516157919 ## CI report: * 031918f2aaa0e81ff10a2270f7e0bf775549bfea Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16782) * a106b790e40138c9d847e09790d24e284a68d678 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3918416479
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -318,12 +316,34 @@ public void close() {
if (metaStoreClient != null) {
Hive.closeCurrent();
}
-if (hiveDriver != null) {
- try {
-hiveDriver.close();
- } finally {
-destroyQuietly(hiveDriver);
+try {
Review Comment:
Done, thanks!
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -176,6 +169,11 @@ private List
updateHiveSQLs(List sqls) {
List responses = new ArrayList<>();
HoodieTimer timer = HoodieTimer.start();
try {
+ // Driver.compile() resolves its session from a thread local that every
executor on this
+ // thread writes: the one constructed most recently wins, and one that
is closed clears it.
+ // Re-assert ours, as Hive documents a thread running several sessions
must, so these
+ // statements run under the session that owns hiveDriver.
+ SessionState.setCurrentSessionState(sessionState);
Review Comment:
Done as well
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5515063105 ## CI report: * 031918f2aaa0e81ff10a2270f7e0bf775549bfea Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16782) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5514647322 ## CI report: * c4edd07f3bd15309959a2b4995f103bcf022d25d Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16753) * 031918f2aaa0e81ff10a2270f7e0bf775549bfea Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16782) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5514564117 ## CI report: * c4edd07f3bd15309959a2b4995f103bcf022d25d Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16753) * 031918f2aaa0e81ff10a2270f7e0bf775549bfea UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
danny0405 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3910567788
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -318,12 +316,34 @@ public void close() {
if (metaStoreClient != null) {
Hive.closeCurrent();
}
-if (hiveDriver != null) {
- try {
-hiveDriver.close();
- } finally {
-destroyQuietly(hiveDriver);
+try {
Review Comment:
[P1] Bind the owned session during Driver teardown
`Driver.close()` and `destroy()` consult `SessionState.get()`—the former
clears the current session’s lineage state, and the latter can obtain its
transaction manager when releasing locks. If executor B was constructed or run
after executor A, calling `A.close()` here tears down A’s Driver while B’s
session is current, potentially clearing or operating on B’s state;
`A.sessionState.close()` then also detaches B. Capture the previous session,
bind `sessionState` before Driver/session teardown, and restore the previous
session afterward.
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -176,6 +169,11 @@ private List
updateHiveSQLs(List sqls) {
List responses = new ArrayList<>();
HoodieTimer timer = HoodieTimer.start();
try {
+ // Driver.compile() resolves its session from a thread local that every
executor on this
+ // thread writes: the one constructed most recently wins, and one that
is closed clears it.
+ // Re-assert ours, as Hive documents a thread running several sessions
must, so these
+ // statements run under the session that owns hiveDriver.
+ SessionState.setCurrentSessionState(sessionState);
Review Comment:
[P2] Restore the previous thread-local session
This assignment remains in the thread local after `updateHiveSQLs` returns.
With interleaved executors—or an embedding application that already owns a Hive
session—subsequent Hive work silently runs under this executor’s database,
configuration, and transaction state. Make the binding scoped: save
`SessionState.get()`, bind this executor for the Driver calls, then restore the
saved session (or detach when it was null) in a `finally` block.
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5501537948 ## CI report: * c4edd07f3bd15309959a2b4995f103bcf022d25d Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16753) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5500746702 ## CI report: * fef87d989dbb8e9b0682349bf9b93e032047c484 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16750) * c4edd07f3bd15309959a2b4995f103bcf022d25d Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16753) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5500579505 ## CI report: * fef87d989dbb8e9b0682349bf9b93e032047c484 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16750) * c4edd07f3bd15309959a2b4995f103bcf022d25d UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5500548319 Hi @danny0405 do you mind taking a look at this fix? Found this issue during the stress testing in Uber and fixed it by this change. Thanks! -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
skywalker0618 commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3908486740
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -318,12 +316,34 @@ public void close() {
if (metaStoreClient != null) {
Hive.closeCurrent();
}
-if (hiveDriver != null) {
- try {
-hiveDriver.close();
- } finally {
-destroyQuietly(hiveDriver);
+try {
+ if (hiveDriver != null) {
+try {
+ hiveDriver.close();
+} finally {
+ destroyQuietly(hiveDriver);
+}
}
+} finally {
+ closeQuietly(sessionState);
+}
+ }
+
+ /**
+ * Closes the SessionState this executor started. Hive derives the session's
four scratch
+ * directory roots from hive.session.id and only reclaims them in close(),
so a HiveSyncTool that
+ * runs per commit leaves a directory set behind on every sync. Runs after
the Driver teardown
+ * above, because close() detaches the session from this thread and {@code
Driver.destroy()} can
+ * reach {@code SessionState.get()} while releasing locks.
+ */
+ private static void closeQuietly(SessionState sessionState) {
+if (sessionState == null) {
+ return;
+}
+try {
+ sessionState.close();
+} catch (Exception e) {
+ log.error("Error while closing SessionState", e);
}
}
Review Comment:
Done
--
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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5498892092 ## CI report: * fef87d989dbb8e9b0682349bf9b93e032047c484 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16750) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
codecov-commenter commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5498168165 ## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19812?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 77.24%. Comparing base ([`2315459`](https://app.codecov.io/gh/apache/hudi/commit/231545998a12fd1943e2393065f76cbdc0602eac?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fef87d9`](https://app.codecov.io/gh/apache/hudi/commit/fef87d989dbb8e9b0682349bf9b93e032047c484?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 50 commits behind head on master. Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #19812 +/- ## - Coverage 77.88% 77.24% -0.64% - Complexity3326533746 +481 Files 2533 2536 +3 Lines140348 143751+3403 Branches 1691317482 +569 + Hits 109304 111045+1741 - Misses2340624927+1521 - Partials 7638 7779 +141 ``` | [Components](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.57% <11.74%> (-1.79%)` | :arrow_down: | | [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.84% <ø> (-1.12%)` | :arrow_down: | | [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.58% <ø> (ø)` | | | [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.37% <ø> (-0.01%)` | :arrow_down: | | [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.33% <ø> (ø)` | | | [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.06% <ø> (ø)` | | | [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.91% <10.71%> (-0.17%)` | :arrow_down: | | [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.61% <ø> (+0.09%)` | :arrow_up: | | [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.76% <ø> (ø)` | | | [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.44% <ø> (ø)` | | | [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.33% <ø> (ø)` | | | [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19812/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | | | [Flag](https://app.codecov.io/gh/apache/hudi/pull/19812/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| |
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5498085257 ## CI report: * fef87d989dbb8e9b0682349bf9b93e032047c484 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16750) Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-bot commented on PR #19812: URL: https://github.com/apache/hudi/pull/19812#issuecomment-5498017911 ## CI report: * fef87d989dbb8e9b0682349bf9b93e032047c484 UNKNOWN Bot commands @hudi-bot supports the following commands: - `@hudi-bot run azure` re-run the last Azure build -- 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]
Re: [PR] fix(hive-sync): close the SessionState HiveQL sync starts [hudi]
hudi-agent commented on code in PR #19812:
URL: https://github.com/apache/hudi/pull/19812#discussion_r3906737830
##
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/HiveQueryDDLExecutor.java:
##
@@ -318,12 +316,34 @@ public void close() {
if (metaStoreClient != null) {
Hive.closeCurrent();
}
-if (hiveDriver != null) {
- try {
-hiveDriver.close();
- } finally {
-destroyQuietly(hiveDriver);
+try {
+ if (hiveDriver != null) {
+try {
+ hiveDriver.close();
+} finally {
+ destroyQuietly(hiveDriver);
+}
}
+} finally {
+ closeQuietly(sessionState);
+}
+ }
+
+ /**
+ * Closes the SessionState this executor started. Hive derives the session's
four scratch
+ * directory roots from hive.session.id and only reclaims them in close(),
so a HiveSyncTool that
+ * runs per commit leaves a directory set behind on every sync. Runs after
the Driver teardown
+ * above, because close() detaches the session from this thread and {@code
Driver.destroy()} can
+ * reach {@code SessionState.get()} while releasing locks.
+ */
+ private static void closeQuietly(SessionState sessionState) {
+if (sessionState == null) {
+ return;
+}
+try {
+ sessionState.close();
+} catch (Exception e) {
+ log.error("Error while closing SessionState", e);
}
}
Review Comment:
🤖 nit: could you rename the parameter to avoid the same name as the instance
field? Something like `state` would make it immediately clear this is a static
helper and not accidentally referencing the field.
⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.
--
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]
