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]
