hudi-agent commented on code in PR #20219:
URL: https://github.com/apache/hudi/pull/20219#discussion_r4254276837


##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveSyncClient.java:
##########
@@ -265,23 +279,11 @@ private Option<HiveDriverPool> 
maybeBuildHiveDriverPool(HiveSyncConfig config) {
     return Option.of(new HiveDriverPool(config, size));
   }
 
-  /**
-   * Builds the (optional) partition-phase pools and the {@link 
HiveQueryDDLExecutor}
-   * that uses them, rolling back whichever pool(s) already got built if a 
later step
-   * in this sequence throws. Without this, a failure in {@code 
maybeBuildPartitionClientPool}
-   * (after {@code partitionDriverPool} was already built) or in the 
executor's own
-   * constructor would leak the already-built pool's worker threads and 
Thrift/Driver
-   * connections, since this constructor's outer catch just rethrows.
-   */
-  private HiveQueryDDLExecutor buildHiveQueryDDLExecutor(HiveSyncConfig 
config) {
-    try {
-      this.partitionDriverPool = maybeBuildHiveDriverPool(config);
-      this.partitionClientPool = maybeBuildPartitionClientPool(config);
-      return new HiveQueryDDLExecutor(config, this.client, 
this.partitionDriverPool, this.partitionClientPool);
-    } catch (Exception e) {
-      closePartitionPoolsQuietly();
-      throw e;
-    }
+  private HiveQueryDDLExecutor buildHiveQueryDDLExecutor() {
+    partitionDriverPool = maybeBuildHiveDriverPool(config);
+    partitionClientPool = maybeBuildPartitionClientPool(config);
+    return new HiveQueryDDLExecutor(config, client, partitionDriverPool, 
partitionClientPool,
+        config.getBooleanOrDefault(HIVE_SYNC_USE_SPARK_CATALOG) ? 
Option.empty() : Option.of(Thread.currentThread()));

Review Comment:
   🤖 nit: could you pull this ternary into a named local, like 
`threadCachingClient`, and add a one-line comment on why the Spark catalog path 
doesn't cache the client on this thread?
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



-- 
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]

Reply via email to