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


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java:
##########
@@ -2350,6 +2350,9 @@ private void invalidate(String bpid, Block[] invalidBlks, 
boolean async)
         removing = volumeMap.remove(bpid, invalidBlks[i]);
         addDeletingBlock(bpid, removing.getBlockId());
         LOG.debug("Block file {} is to be deleted", removing.getBlockURI());
+        if (datanode.getMetrics() != null) {

Review Comment:
   @ZanderXu Sorry to pollute this flow. After review this context carefully, I 
realize `removing` will never be null actually. It will continue when meets 
null, and will never go to this logic (ref to Line2317).  Another one, it is 
unnecessary to decide if `datanode.getMetrics()` is null or not. The first 
version is without any issues.
   I have made a common mistake "ask a programmer to review 10 lines of code 
he'll find 10 issues, ask him to do 500 lines and he'll say it looks good."(via 
twitter)
   "Wish every PR could be merged without any issues reviewers feedback." ^_^



-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to