Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2754#discussion_r209152808
--- Diff:
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogCleaner.java
---
@@ -142,30 +154,41 @@ public void run() {
oldLogDirs.stream().map(File::getName).collect(joining(",")),
deadWorkerDirs.stream().map(File::getName).collect(joining(",")));
- deadWorkerDirs.forEach(Unchecked.consumer(dir -> {
+ for (File dir : deadWorkerDirs) {
String path = dir.getCanonicalPath();
- LOG.info("Cleaning up: Removing {}", path);
+ long size = FileUtils.sizeOf(dir);
+ LOG.info("Cleaning up: Removing {}, {} KB", path, size *
1e-3);
try {
Utils.forceDelete(path);
cleanupEmptyTopoDirectory(dir);
+ numFilesCleaned++;
+ diskSpaceCleaned += size;
} catch (Exception ex) {
+ ExceptionMeters.NUM_FILE_REMOVAL_EXCEPTIONS.mark();
LOG.error(ex.getMessage(), ex);
}
- }));
+ }
- perWorkerDirCleanup(maxPerWorkerLogsSizeMb * 1024 * 1024);
- globalLogCleanup(maxSumWorkerLogsSizeMb * 1024 * 1024);
+ final List<DirectoryCleaner.DeletionMeta>
perWorkerDirCleanupMeta = perWorkerDirCleanup(maxPerWorkerLogsSizeMb * 1024 *
1024);
+ numFilesCleaned +=
perWorkerDirCleanupMeta.stream().mapToInt(meta -> meta.deletedFiles).sum();
+ diskSpaceCleaned +=
perWorkerDirCleanupMeta.stream().mapToLong(meta -> meta.deletedSize).sum();
+ final DirectoryCleaner.DeletionMeta globalLogCleanupMeta =
globalLogCleanup(maxSumWorkerLogsSizeMb * 1024 * 1024);
+ numFilesCleaned += globalLogCleanupMeta.deletedFiles;
+ diskSpaceCleaned += globalLogCleanupMeta.deletedSize;
} catch (Exception ex) {
+ ExceptionMeters.NUM_CLEANUP_EXCEPTIONS.mark();
LOG.error("Exception while cleaning up old log.", ex);
}
+ numFilesCleanedUp.update(numFilesCleaned);
+ diskSpaceFreed.update(diskSpaceCleaned);
}
/**
* Delete the oldest files in each overloaded worker log dir.
*/
@VisibleForTesting
- List<Integer> perWorkerDirCleanup(long size) {
+ List<DirectoryCleaner.DeletionMeta> perWorkerDirCleanup(long size) {
--- End diff --
I believe commons lang has a tuple class, but tbh I think keeping this
class is better for readability (otherwise we'd get the same situation as the
list in ExecutorDetails).
---
