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]
