xinglin commented on code in PR #6290:
URL: https://github.com/apache/hadoop/pull/6290#discussion_r1414312222
##########
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:
thanks @Hexiaoqiao for your comment. added a check for negative transfer
bytes and replace it with 0 byte.
--
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]