[jira] [Commented] (HDFS-17262) Transfer rate metric warning log is too verbose

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HDFS-17262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17792516#comment-17792516 ] 

ASF GitHub Bot commented on HDFS-17262:
---------------------------------------

Hexiaoqiao commented on code in PR #6290:
URL: https://github.com/apache/hadoop/pull/6290#discussion_r1413054649


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java:
##########
@@ -1970,16 +1971,32 @@ public static boolean isParentEntry(final String path, final String parent) {
   }
 
   /**
-   * Add transfer rate metrics for valid data read and duration values.
+   * Add transfer rate metrics in bytes per second.
    * @param metrics metrics for datanodes
    * @param read bytes read
-   * @param duration read duration
+   * @param durationInNS read duration in nanoseconds
    */
-  public static void addTransferRateMetric(final DataNodeMetrics metrics, final long read, final long duration) {
-    if (read >= 0 && duration > 0) {
-        metrics.addReadTransferRate(read * 1000 / duration);
-    } else {
-      LOG.warn("Unexpected value for data transfer bytes={} duration={}", read, duration);
-    }
+  public static void addTransferRateMetric(final DataNodeMetrics metrics, final long read,
+      final long durationInNS) {
+    metrics.addReadTransferRate(getTransferRateInBytesPerSecond(read, durationInNS));
+  }
+
+  /**
+   * Calculate the transfer rate in bytes per second.
+   *
+   * We have the read duration in nanoseconds for precision for transfers taking a few nanoseconds.
+   * We treat shorter durations below 1 ns as 1 ns as we also want to capture reads taking less
+   * than a nanosecond. To calculate transferRate in bytes per second, we avoid multiplying bytes
+   * read by 10^9 to avoid overflow. Instead, we first calculate the duration in seconds in double
+   * to keep the decimal values for smaller durations. We then divide bytes read by
+   * durationInSeconds to get the transferRate in bytes per second.
+   * @param bytes bytes read
+   * @param durationInNS read duration in nanoseconds
+   * @return bytes per second
+   */
+  public static long getTransferRateInBytesPerSecond(final long bytes, long durationInNS) {
+    durationInNS = Math.max(durationInNS, 1);
+    double durationInSeconds = (double) durationInNS / TimeUnit.SECONDS.toNanos(1);
+    return (long) (bytes / durationInSeconds);

Review Comment:
   How about to guard `bytes` should be one positive number?





> Transfer rate metric warning log is too verbose
> -----------------------------------------------
>
>                 Key: HDFS-17262
>                 URL: https://issues.apache.org/jira/browse/HDFS-17262
>             Project: Hadoop HDFS
>          Issue Type: Bug
>            Reporter: Bryan Beaudreault
>            Assignee: Xing Lin
>            Priority: Major
>              Labels: pull-request-available
>
> HDFS-16917 added a LOG.warn when passed duration is 0. The unit for duration is millis, and its very possible for a read to take less than a millisecond when considering local TCP connection. We are seeing this spam multiple times per millisecond. There's another report on the PR for HDFS-16917.
> Please downgrade to debug or remove the log



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org