dengzhhu653 commented on a change in pull request #2441:
URL: https://github.com/apache/hive/pull/2441#discussion_r683067748



##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java
##########
@@ -202,6 +215,8 @@ public Result invokeInternal(final Object proxy, final 
Method method, final Obje
           LOG.error(ExceptionUtils.getStackTrace(e.getCause()));
           throw e.getCause();
         }
+      } finally {
+        endFunction(method, object, ex, args);

Review comment:
       The `startFunctions` takes care of these:
   1.  API Metrics,  which is duplicated with 
[PerfLogger](https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/PerfLogger.java#L188-L207)
 in RetryingHMSHandler, this part cloud be elimated from the `startFunctions`.
   2. Audit Logs, it is difficult to move these to a standalone function, as 
the log is related with the input of the method. For example, we have two 
`startPartitionFunction` for the partition related methods:
   ```
     private void startPartitionFunction(String function, String cat, String 
db, String tbl,
                                         List<String> partVals) {
       startFunction(function, " : tbl=" +
           TableName.getQualified(cat, db, tbl) + "[" + join(partVals, ",") + 
"]");
     }
   
     private void startPartitionFunction(String function, String catName, 
String db, String tbl,
                                         Map<String, String> partName) {
       startFunction(function, " : tbl=" +
           TableName.getQualified(catName, db, tbl) + "partition=" + partName);
     }
   ```
   
   In some cases, we also use only log the table name by using 
`startTableFunction`, like 
[get_partitions](https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java#L5453)
   These functions make the audit log of partitions different, another concern 
is that if we add/remove a method, the audit log should be changed elsewhere, 
this may be upsetting.
   




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to