haiyang1987 commented on code in PR #6559:
URL: https://github.com/apache/hadoop/pull/6559#discussion_r1494250286


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java:
##########
@@ -2400,37 +2407,42 @@ public void invalidate(String bpid, ReplicaInfo block) {
   }
   /**
    * Invalidate a block which is not found on disk. We should remove it from
-   * memory and notify namenode, but unnecessary  to delete the actual on-disk
-   * block file again.
+   * memory and notify namenode, will decide whether to delete the actual 
on-disk block and meta
+   * file based on {@link 
DFSConfigKeys#DFS_DATANODE_DELETE_CORRUPT_REPLICA_FROM_DISK_ENABLE}.
    *
    * @param bpid the block pool ID.
    * @param block The block to be invalidated.
    * @param checkFiles Whether to check data and meta files.
    */
-  public void invalidateMissingBlock(String bpid, Block block, boolean 
checkFiles) {
+  public void invalidateMissingBlock(String bpid, Block block, boolean 
checkFiles)
+      throws IOException {
 
     // The replica seems is on its volume map but not on disk.
     // We can't confirm here is block file lost or disk failed.
-    // If block lost:
-    //    deleted local block file is completely unnecessary
-    // If disk failed:
-    //    deleted local block file here may lead to missing-block
-    //    when it with only 1 replication left now.
-    // So remove if from volume map notify namenode is ok.
+    // If checkFiles as true will check block or meta file existence again.
+    // If deleteCorruptReplicaFromDisk as true will delete the actual on-disk 
block and meta file,
+    // otherwise will remove it from volume map and notify namenode.
     try (AutoCloseableLock lock = lockManager.writeLock(LockLevel.BLOCK_POOl,
         bpid)) {
       // Check if this block is on the volume map.
       ReplicaInfo replica = volumeMap.get(bpid, block);
       // Double-check block or meta file existence when checkFiles as true.
       if (replica != null && (!checkFiles ||
           (!replica.blockDataExists() || !replica.metadataExists()))) {
-        volumeMap.remove(bpid, block);
-        invalidate(bpid, replica);
+        if (deleteCorruptReplicaFromDisk) {
+          ExtendedBlock extendedBlock = new ExtendedBlock(bpid, block);
+          datanode
+              .notifyNamenodeDeletedBlock(extendedBlock, 
replica.getStorageUuid());
+          invalidate(bpid, new Block[] {extendedBlock.getLocalBlock()});
+        } else {
+          volumeMap.remove(bpid, block);

Review Comment:
   Modify patch based on comments.
   
   Hi @ZanderXu please help review it again, thanks~



-- 
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