You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2023/03/02 07:00:48 UTC

[impala] branch master updated: IMPALA-11948: Remove operation logging from getCatalogServerMetrics

This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new eaf71bca0 IMPALA-11948: Remove operation logging from getCatalogServerMetrics
eaf71bca0 is described below

commit eaf71bca0a3538056765dbbce13c0f503045b38b
Author: Peter Rozsa <pr...@cloudera.com>
AuthorDate: Tue Feb 28 08:44:07 2023 +0100

    IMPALA-11948: Remove operation logging from getCatalogServerMetrics
    
    IMPALA-11478 added operation-level logging to every catalog method in
    JniCatalog, but getCatalogServerMetrics operation is called many times
    by catalogd itself. Start and finish logs for this operation are useless,
    therefore only the warnings about size and elapsed time should remain.
    
    Tests:
       - manually validated that warning-level logs are still generated
       - manually validated that start/finish log entries are not generated
    
    Change-Id: Ifd01e9f42ccc6a4adce41caf8884ea3c2c781452
    Reviewed-on: http://gerrit.cloudera.org:8080/19557
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../java/org/apache/impala/common/JniUtil.java     | 34 +++++++++++++++++-----
 .../java/org/apache/impala/service/JniCatalog.java | 21 +++++++++----
 .../org/apache/impala/service/JniCatalogOp.java    | 30 +++++++++++++++----
 3 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/common/JniUtil.java b/fe/src/main/java/org/apache/impala/common/JniUtil.java
index 43b09bc99..6835a1d75 100644
--- a/fe/src/main/java/org/apache/impala/common/JniUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/JniUtil.java
@@ -147,19 +147,28 @@ public class JniUtil {
     private final long startTime;
     private final String methodName;
     private final String shortDescription;
+    private final boolean silentStartAndFinish;
 
-    public OperationLog(String methodName, String shortDescription) {
+    public OperationLog(
+        String methodName, String shortDescription, boolean silentStartAndFinish) {
       this.startTime = System.currentTimeMillis();
       this.methodName = methodName;
       this.shortDescription = shortDescription;
+      this.silentStartAndFinish = silentStartAndFinish;
     }
 
-    public void logStart() { LOG.info("{} request: {}", methodName, shortDescription); }
+    public void logStart() {
+      if (!silentStartAndFinish) {
+        LOG.info("{} request: {}", methodName, shortDescription);
+      }
+    }
 
     public void logFinish() {
-      long duration = getDurationFromStart();
-      LOG.info("Finished {} request: {}. Time spent: {}", methodName, shortDescription,
-          PrintUtils.printTimeMs(duration));
+      if (!silentStartAndFinish) {
+        long duration = getDurationFromStart();
+        LOG.info("Finished {} request: {}. Time spent: {}", methodName, shortDescription,
+            PrintUtils.printTimeMs(duration));
+      }
     }
 
     public void logError() {
@@ -195,12 +204,23 @@ public class JniUtil {
     }
   }
 
-  public static OperationLog logOperation(String methodName, String shortDescription) {
-    OperationLog operationLog = new OperationLog(methodName, shortDescription);
+  private static OperationLog logOperationInternal(
+      String methodName, String shortDescription, boolean silentStartAndFinish) {
+    OperationLog operationLog =
+        new OperationLog(methodName, shortDescription, silentStartAndFinish);
     operationLog.logStart();
     return operationLog;
   }
 
+  public static OperationLog logOperation(String methodName, String shortDescription) {
+    return logOperationInternal(methodName, shortDescription, false);
+  }
+
+  public static OperationLog logOperationSilentStartAndFinish(
+      String methodName, String shortDescription) {
+    return logOperationInternal(methodName, shortDescription, true);
+  }
+
   /**
    * Collect the JVM's memory statistics into a thrift structure for translation into
    * Impala metrics by the backend. A synthetic 'total' memory pool is included with
diff --git a/fe/src/main/java/org/apache/impala/service/JniCatalog.java b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
index 9b7abff12..8d3488e5c 100644
--- a/fe/src/main/java/org/apache/impala/service/JniCatalog.java
+++ b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
@@ -231,6 +231,14 @@ public class JniCatalog {
         methodName, shortDescription, operand, serializer, finalClause);
   }
 
+  private byte[] execAndSerializeSilentStartAndFinish(String methodName,
+      String shortDescription, JniCatalogOpCallable<TBase<?, ?>> operand)
+      throws ImpalaException, TException {
+    TSerializer serializer = new TSerializer(protocolFactory_);
+    return JniCatalogOp.execAndSerializeSilentStartAndFinish(
+        methodName, shortDescription, operand, serializer, () -> {});
+  }
+
   private byte[] execAndSerialize(String methodName, String shortDescription,
       JniCatalogOpCallable<TBase<?, ?>> operand) throws ImpalaException, TException {
     return execAndSerialize(methodName, shortDescription, operand, () -> {});
@@ -505,11 +513,12 @@ public class JniCatalog {
   public byte[] getCatalogServerMetrics() throws ImpalaException, TException {
     TGetCatalogServerMetricsResponse response = new TGetCatalogServerMetricsResponse();
     String shortDesc = "Get catalog server metrics";
-    return execAndSerialize("getCatalogServerMetrics", shortDesc, () -> {
-      response.setCatalog_partial_fetch_rpc_queue_len(
-          catalog_.getPartialFetchRpcQueueLength());
-      response.setEvent_metrics(catalog_.getEventProcessorMetrics());
-      return response;
-    });
+    return execAndSerializeSilentStartAndFinish(
+        "getCatalogServerMetrics", shortDesc, () -> {
+          response.setCatalog_partial_fetch_rpc_queue_len(
+              catalog_.getPartialFetchRpcQueueLength());
+          response.setEvent_metrics(catalog_.getEventProcessorMetrics());
+          return response;
+        });
   }
 }
diff --git a/fe/src/main/java/org/apache/impala/service/JniCatalogOp.java b/fe/src/main/java/org/apache/impala/service/JniCatalogOp.java
index 6db6f8124..8e48bd6c2 100644
--- a/fe/src/main/java/org/apache/impala/service/JniCatalogOp.java
+++ b/fe/src/main/java/org/apache/impala/service/JniCatalogOp.java
@@ -49,13 +49,11 @@ public class JniCatalogOp {
    */
   public static <RESULT, PARAMETER extends TBase<?, ?>> RESULT execOp(String methodName,
       String shortDescription, JniCatalogOpCallable<Pair<RESULT, Long>> operand,
-      PARAMETER requestParameter, Runnable finalClause)
+      PARAMETER requestParameter, Runnable finalClause, OperationLog operationLog)
       throws ImpalaException, TException {
     Preconditions.checkNotNull(operand);
     Preconditions.checkNotNull(finalClause);
 
-    OperationLog operationLog = JniUtil.logOperation(methodName, shortDescription);
-
     try (ThreadNameAnnotator ignored = new ThreadNameAnnotator(shortDescription)) {
       Pair<RESULT, Long> result = operand.call();
 
@@ -79,16 +77,36 @@ public class JniCatalogOp {
   public static <RESULT, PARAMETER extends TBase<?, ?>> RESULT execOp(String methodName,
       String shortDescription, JniCatalogOpCallable<Pair<RESULT, Long>> operand,
       PARAMETER parameter) throws ImpalaException, TException {
-    return execOp(methodName, shortDescription, operand, parameter, () -> {});
+    OperationLog operationLog = JniUtil.logOperation(methodName, shortDescription);
+    return execOp(
+        methodName, shortDescription, operand, parameter, () -> {}, operationLog);
   }
 
   public static byte[] execAndSerialize(String methodName, String shortDescription,
       JniCatalogOpCallable<TBase<?, ?>> operand, TSerializer serializer,
-      Runnable finalClause) throws ImpalaException, TException {
+      Runnable finalClause, OperationLog operationLog)
+      throws ImpalaException, TException {
     return execOp(methodName, shortDescription, () -> {
       TBase<?, ?> response = operand.call();
       byte[] result = serializer.serialize(response);
       return Pair.create(result, (long) result.length);
-    }, null, finalClause);
+    }, null, finalClause, operationLog);
+  }
+
+  public static byte[] execAndSerialize(String methodName, String shortDescription,
+      JniCatalogOpCallable<TBase<?, ?>> operand, TSerializer serializer,
+      Runnable finalClause) throws ImpalaException, TException {
+    OperationLog operationLog = JniUtil.logOperation(methodName, shortDescription);
+    return execAndSerialize(
+        methodName, shortDescription, operand, serializer, finalClause, operationLog);
+  }
+
+  public static byte[] execAndSerializeSilentStartAndFinish(String methodName,
+      String shortDescription, JniCatalogOpCallable<TBase<?, ?>> operand,
+      TSerializer serializer, Runnable finalClause) throws ImpalaException, TException {
+    OperationLog operationLog =
+        JniUtil.logOperationSilentStartAndFinish(methodName, shortDescription);
+    return execAndSerialize(
+        methodName, shortDescription, operand, serializer, finalClause, operationLog);
   }
 }