-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-17273. Improve local variables duration of DataStreamer for better debugging. #6321
Conversation
… for better debugging
@Hexiaoqiao @tomscut Sir, could you please help me review this simple modification when you have free time? Thanks ahead. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
Show resolved
Hide resolved
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
Show resolved
Hide resolved
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
Show resolved
Hide resolved
LOG.error("No ack received, took {}ms (threshold={}ms). " | ||
+ "File being written: {}, block: {}, " | ||
+ "Write pipeline datanodes: {}.", | ||
duration / NANOSECONDS_PER_MILLISECOND, writeTimeout, src, block, nodes); | ||
TimeUnit.NANOSECONDS.toSeconds(duration), writeTimeout, src, block, nodes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TimeUnit.NANOSECONDS.toMillis(duration) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it, thanks sir.
8470a55
to
75bd7b0
Compare
LGTM +1 |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1.
Committed to trunk. Thanks @hfutatzhanghb , @haiyang1987 and @tomscut . |
@Hexiaoqiao Sir, thanks a lot for your merging and reviewing. Thanks @haiyang1987 @tomscut for reviewing carefully. |
… for better debugging (apache#6321). Contributed by farmmamba. Reviewed-by: Haiyang Hu <[email protected]> Reviewed-by: Tao Li <[email protected]> Signed-off-by: He Xiaoqiao <[email protected]>
Description of PR
Since Time.monotonicNow() use System.nanoTime() / 1000,it maybe return 0 when we are debugging and make us confused.
So, we'd better change the way of computing duration.