Migrate existing fucntions to adapt runtime metric unit

functions now adopt the runtime unit in their signature rather than choosing for them
prestodb · May 20, 2022 · a9037be · a9037be
1 parent e2c1132
commit a9037be
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 21 deletions.
diff --git a/presto-cache/src/main/java/com/facebook/presto/cache/alluxio/PrestoCacheContext.java b/presto-cache/src/main/java/com/facebook/presto/cache/alluxio/PrestoCacheContext.java
@@ -19,6 +19,7 @@
 import com.facebook.presto.hive.HiveFileContext;
 import com.google.common.collect.ImmutableMap;
 
+import static com.facebook.presto.common.RuntimeUnit.NANO;
 import static java.util.Objects.requireNonNull;
 
 public class PrestoCacheContext
@@ -50,7 +51,7 @@ private PrestoCacheContext(HiveFileContext hiveFileContext)
     @Override
     public void incrementCounter(String name, long value)
     {
-        hiveFileContext.incrementCounter(name, value);
+        hiveFileContext.incrementCounter(name, NANO, value);
     }
 
     public HiveFileContext getHiveFileContext()

diff --git a/presto-cli/src/main/java/com/facebook/presto/cli/StatusPrinter.java b/presto-cli/src/main/java/com/facebook/presto/cli/StatusPrinter.java
@@ -19,6 +19,7 @@
 import com.facebook.presto.client.StatementClient;
 import com.facebook.presto.client.StatementStats;
 import com.facebook.presto.common.RuntimeMetric;
+import com.facebook.presto.common.RuntimeUnit;
 import com.google.common.base.Strings;
 import com.google.common.primitives.Ints;
 import io.airlift.units.DataSize;
@@ -150,14 +151,15 @@ private void updateScreen(WarningsPrinter warningsPrinter)
         printQueryInfo(client.currentStatusInfo(), warningsPrinter);
     }
 
-    private String autoFormatMetricValue(String name, long value)
+    private String autoFormatMetricValue(RuntimeUnit unit, long value)
     {
-        // TODO: The current implementation of detecting metric type is hacky. An better solution is to add an enum to RuntimeMetric to track its type. But since there are no
-        // other use cases, it is too heavy to implement. We can revisit this when there are new use cases.
-        if (name.contains("Nanos")) {
+        if (unit == RuntimeUnit.NANO) {
             return formatTime(Duration.succinctNanos(value));
         }
-        return formatCount(value);
+        if (unit == RuntimeUnit.BYTE) {
+            return formatDataSize(bytes(value), true);
+        }
+        return formatCount(value); // NONE
     }
 
     public void printFinalInfo()
@@ -233,10 +235,10 @@ public void printFinalInfo()
                 stats.getRuntimeStats().getMetrics().values().stream().sorted(Comparator.comparing(RuntimeMetric::getName)).forEach(
                         metric -> reprintLine(format("%s: sum=%s count=%s min=%s max=%s",
                                 metric.getName(),
-                                autoFormatMetricValue(metric.getName(), metric.getSum()),
+                                autoFormatMetricValue(metric.getUnit(), metric.getSum()),
                                 formatCount(metric.getCount()),
-                                autoFormatMetricValue(metric.getName(), metric.getMin()),
-                                autoFormatMetricValue(metric.getName(), metric.getMax()))));
+                                autoFormatMetricValue(metric.getUnit(), metric.getMin()),
+                                autoFormatMetricValue(metric.getUnit(), metric.getMax()))));
             }
         }
 
@@ -343,10 +345,10 @@ private void printQueryInfo(QueryStatusInfo results, WarningsPrinter warningsPri
                     stats.getRuntimeStats().getMetrics().values().stream().sorted(Comparator.comparing(RuntimeMetric::getName)).forEach(
                             metric -> reprintLine(format("%s: sum=%s count=%s min=%s max=%s",
                                     metric.getName(),
-                                    autoFormatMetricValue(metric.getName(), metric.getSum()),
+                                    autoFormatMetricValue(metric.getUnit(), metric.getSum()),
                                     formatCount(metric.getCount()),
-                                    autoFormatMetricValue(metric.getName(), metric.getMin()),
-                                    autoFormatMetricValue(metric.getName(), metric.getMax()))));
+                                    autoFormatMetricValue(metric.getUnit(), metric.getMin()),
+                                    autoFormatMetricValue(metric.getUnit(), metric.getMax()))));
                 }
             }
 

diff --git a/presto-hive-common/src/main/java/com/facebook/presto/hive/HiveFileContext.java b/presto-hive-common/src/main/java/com/facebook/presto/hive/HiveFileContext.java
@@ -14,10 +14,10 @@
 package com.facebook.presto.hive;
 
 import com.facebook.presto.common.RuntimeStats;
+import com.facebook.presto.common.RuntimeUnit;
 
 import java.util.Optional;
 
-import static com.facebook.presto.common.RuntimeUnit.NONE;
 import static com.facebook.presto.hive.CacheQuota.NO_CACHE_CONSTRAINTS;
 import static java.util.Objects.requireNonNull;
 
@@ -80,10 +80,10 @@ public interface ExtraHiveFileInfo<T>
         T getExtraFileInfo();
     }
 
-    public void incrementCounter(String name, long value)
+    public void incrementCounter(String name, RuntimeUnit unit, long value)
     {
         if (verboseRuntimeStatsEnabled) {
-            stats.addMetricValue(name, NONE, value);
+            stats.addMetricValue(name, unit, value);
         }
     }
 

diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/parquet/ParquetPageSourceFactory.java b/presto-hive/src/main/java/com/facebook/presto/hive/parquet/ParquetPageSourceFactory.java
@@ -75,6 +75,8 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import static com.facebook.presto.common.RuntimeUnit.BYTE;
+import static com.facebook.presto.common.RuntimeUnit.NONE;
 import static com.facebook.presto.common.type.StandardTypes.ARRAY;
 import static com.facebook.presto.common.type.StandardTypes.BIGINT;
 import static com.facebook.presto.common.type.StandardTypes.CHAR;
@@ -283,14 +285,14 @@ public static ConnectorPageSource createParquetPageSource(
                     blocks.add(block);
                     blockStarts.add(nextStart);
                     blockIndexStores.add(columnIndexStore.orElse(null));
-                    hiveFileContext.incrementCounter("parquet.blocksRead", 1);
-                    hiveFileContext.incrementCounter("parquet.rowsRead", block.getRowCount());
-                    hiveFileContext.incrementCounter("parquet.totalBytesRead", block.getTotalByteSize());
+                    hiveFileContext.incrementCounter("parquet.blocksRead", NONE, 1);
+                    hiveFileContext.incrementCounter("parquet.rowsRead", NONE, block.getRowCount());
+                    hiveFileContext.incrementCounter("parquet.totalBytesRead", BYTE, block.getTotalByteSize());
                 }
                 else {
-                    hiveFileContext.incrementCounter("parquet.blocksSkipped", 1);
-                    hiveFileContext.incrementCounter("parquet.rowsSkipped", block.getRowCount());
-                    hiveFileContext.incrementCounter("parquet.totalBytesSkipped", block.getTotalByteSize());
+                    hiveFileContext.incrementCounter("parquet.blocksSkipped", NONE, 1);
+                    hiveFileContext.incrementCounter("parquet.rowsSkipped", NONE, block.getRowCount());
+                    hiveFileContext.incrementCounter("parquet.totalBytesSkipped", BYTE, block.getTotalByteSize());
                 }
                 nextStart += block.getRowCount();
             }