vinothchandar commented on a change in pull request #2759:
URL: https://github.com/apache/hudi/pull/2759#discussion_r616223548



##########
File path: 
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveClient.java
##########
@@ -69,6 +70,8 @@
   private static final Logger LOG = 
LogManager.getLogger(HoodieHiveClient.class);
   private final PartitionValueExtractor partitionValueExtractor;
   private IMetaStoreClient client;
+  private SessionState sessionState;
+  private org.apache.hadoop.hive.ql.Driver hiveDriver;

Review comment:
       Not really an issue from this PR. but do we need to really specify the 
full package name? `org.apache.hadoop.hive.ql.Driver`?

##########
File path: 
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveClient.java
##########
@@ -386,39 +408,18 @@ public CommandProcessorResponse 
updateHiveSQLUsingHiveDriver(String sql) {
   }
 
   private List<CommandProcessorResponse> updateHiveSQLs(List<String> sqls) {
-    SessionState ss = null;
-    org.apache.hadoop.hive.ql.Driver hiveDriver = null;
     List<CommandProcessorResponse> responses = new ArrayList<>();
     try {
-      final long startTime = System.currentTimeMillis();
-      ss = SessionState.start(configuration);
-      ss.setCurrentDatabase(syncConfig.databaseName);
-      hiveDriver = new org.apache.hadoop.hive.ql.Driver(configuration);
-      final long endTime = System.currentTimeMillis();
-      LOG.info(String.format("Time taken to start SessionState and create 
Driver: %s ms", (endTime - startTime)));
       for (String sql : sqls) {
-        final long start = System.currentTimeMillis();
-        responses.add(hiveDriver.run(sql));
-        final long end = System.currentTimeMillis();
-        LOG.info(String.format("Time taken to execute [%s]: %s ms", sql, (end 
- start)));
+        if (hiveDriver != null) {
+          final long start = System.currentTimeMillis();

Review comment:
       lets use `HoodieTimer`? 

##########
File path: 
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveClient.java
##########
@@ -88,8 +91,27 @@ public HoodieHiveClient(HiveSyncConfig cfg, HiveConf 
configuration, FileSystem f
       createHiveConnection();
     }
     try {
+      final long startTime = System.currentTimeMillis();
+      this.sessionState = new SessionState(configuration,
+              UserGroupInformation.getCurrentUser().getShortUserName());
+      SessionState.start(this.sessionState);
+      this.sessionState.setCurrentDatabase(syncConfig.databaseName);
+      hiveDriver = new org.apache.hadoop.hive.ql.Driver(configuration);
       this.client = Hive.get(configuration).getMSC();
-    } catch (MetaException | HiveException e) {
+      final long endTime = System.currentTimeMillis();

Review comment:
       can we use `HoodieTimer` for timing?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to