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

ASF GitHub Bot commented on FLINK-7897:
---------------------------------------

Github user yanghua commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5777#discussion_r177777396
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/blob/TransientBlobCleanupTask.java
 ---
    @@ -100,9 +102,15 @@ public void run() {
                                writeLock.lock();
     
                                try {
    -                                   if (!localFile.delete() && 
localFile.exists()) {
    -                                           log.warn("Failed to locally 
delete blob " + localFile.getAbsolutePath());
    -                                   } else {
    +                                   try {
    --- End diff --
    
    @yuqi1129 I think there is no wrong about the changes. Considering the 
original code, if one of the two conditions : `localFile.delete()` returns 
`true` or _file does not exist_ matched. The code `entries.remove(entry);` will 
be invoked. That means if there is no file (no mater it has been deleted or 
really not exists) , the entries can only be removed.
    
    My changes make sure that if there is no file, the entries will be removed. 
if the file exists it will retained. The `Files.delete` in nio package just 
throw exception but not return bool value. And the original `File#delete` can 
return bool value and throw exception. You can read specific javadoc.



> Consider using nio.Files for file deletion in TransientBlobCleanupTask
> ----------------------------------------------------------------------
>
>                 Key: FLINK-7897
>                 URL: https://issues.apache.org/jira/browse/FLINK-7897
>             Project: Flink
>          Issue Type: Improvement
>          Components: Local Runtime
>            Reporter: Ted Yu
>            Assignee: vinoyang
>            Priority: Minor
>
> nio.Files#delete() provides better clue as to why the deletion may fail:
> https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#delete(java.nio.file.Path)
> Depending on the potential exception (FileNotFound), the call to 
> localFile.exists() may be skipped.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to