Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2788#discussion_r207327029 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/DirectoryCleaner.java --- @@ -133,10 +123,15 @@ public int compare(File f1, File f2) { } while (!stack.isEmpty() && toDeleteSize > 0) { File file = stack.pop(); - toDeleteSize -= file.length(); - LOG.info("Delete file: {}, size: {}, lastModified: {}", file.getCanonicalPath(), file.length(), file.lastModified()); - file.delete(); - deletedFiles++; + final String canonicalPath = file.getCanonicalPath(); + final long fileSize = file.length(); + final long lastModified = file.lastModified(); + //Original implementation doesn't actually check if delete succeeded or not. + if (file.delete()) { --- End diff -- I agree that we should make this check, but won't this change make the outer while loop run again on the same files if the files couldn't be deleted?
---