From 774c1f65524fd753cebbcbe4216a5daca64907ee Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Sun, 20 Jul 2025 08:51:16 +0100 Subject: [PATCH] feat: optimize temp cleanup and add async execution --- .../common/config/CleanupAsyncConfig.java | 24 +++++ .../common/model/ApplicationProperties.java | 2 + .../service/TempFileCleanupService.java | 88 +++++++++++-------- .../service/TempFileCleanupServiceTest.java | 79 +++++++++++------ .../src/main/resources/settings.yml.template | 2 + 5 files changed, 133 insertions(+), 62 deletions(-) create mode 100644 app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java diff --git a/app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java b/app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java new file mode 100644 index 000000000..419451f36 --- /dev/null +++ b/app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java @@ -0,0 +1,24 @@ +package stirling.software.common.config; + +import java.util.concurrent.Executor; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableAsync; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; + +@Configuration +@EnableAsync +public class CleanupAsyncConfig { + + @Bean(name = "cleanupExecutor") + public Executor cleanupExecutor() { + ThreadPoolTaskExecutor exec = new ThreadPoolTaskExecutor(); + exec.setCorePoolSize(1); + exec.setMaxPoolSize(1); + exec.setQueueCapacity(100); + exec.setThreadNamePrefix("cleanup-"); + exec.initialize(); + return exec; + } +} diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index e4edf2baa..fec667e33 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -328,6 +328,8 @@ public class ApplicationProperties { private long cleanupIntervalMinutes = 30; private boolean startupCleanup = true; private boolean cleanupSystemTemp = false; + private int batchSize = 0; + private long pauseBetweenBatchesMs = 0; public String getBaseTmpDir() { return baseTmpDir != null && !baseTmpDir.isEmpty() diff --git a/app/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java b/app/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java index df85a016b..82cafc879 100644 --- a/app/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java +++ b/app/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java @@ -9,10 +9,10 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.function.Predicate; -import java.util.stream.Stream; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; @@ -121,6 +121,7 @@ public class TempFileCleanupService { } /** Scheduled task to clean up old temporary files. Runs at the configured interval. */ + @Async("cleanupExecutor") @Scheduled( fixedDelayString = "#{applicationProperties.system.tempFileManagement.cleanupIntervalMinutes}", @@ -310,44 +311,61 @@ public class TempFileCleanupService { } java.util.List subdirectories = new java.util.ArrayList<>(); + int batchSize = applicationProperties.getSystem().getTempFileManagement().getBatchSize(); + long pauseMs = + applicationProperties + .getSystem() + .getTempFileManagement() + .getPauseBetweenBatchesMs(); + int processed = 0; - try (Stream pathStream = Files.list(directory)) { - pathStream.forEach( - path -> { + try (java.nio.file.DirectoryStream stream = Files.newDirectoryStream(directory)) { + for (Path path : stream) { + try { + String fileName = path.getFileName().toString(); + + if (SHOULD_SKIP.test(fileName)) { + continue; + } + + if (Files.isDirectory(path)) { + subdirectories.add(path); + continue; + } + + if (registry.contains(path.toFile())) { + continue; + } + + if (shouldDeleteFile(path, fileName, containerMode, maxAgeMillis)) { try { - String fileName = path.getFileName().toString(); - - if (SHOULD_SKIP.test(fileName)) { - return; + Files.deleteIfExists(path); + onDeleteCallback.accept(path); + } catch (IOException e) { + if (e.getMessage() != null + && e.getMessage().contains("being used by another process")) { + log.debug("File locked, skipping delete: {}", path); + } else { + log.warn("Failed to delete temp file: {}", path, e); } - - if (Files.isDirectory(path)) { - subdirectories.add(path); - return; - } - - if (registry.contains(path.toFile())) { - return; - } - - if (shouldDeleteFile(path, fileName, containerMode, maxAgeMillis)) { - try { - Files.deleteIfExists(path); - onDeleteCallback.accept(path); - } catch (IOException e) { - if (e.getMessage() != null - && e.getMessage() - .contains("being used by another process")) { - log.debug("File locked, skipping delete: {}", path); - } else { - log.warn("Failed to delete temp file: {}", path, e); - } - } - } - } catch (Exception e) { - log.warn("Error processing path: {}", path, e); } - }); + } + } catch (Exception e) { + log.warn("Error processing path: {}", path, e); + } + + processed++; + if (batchSize > 0 && processed >= batchSize) { + if (pauseMs > 0) { + try { + Thread.sleep(pauseMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + processed = 0; + } + } } for (Path subdirectory : subdirectories) { diff --git a/app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java b/app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java index 34c471227..31db069b9 100644 --- a/app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java +++ b/app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java @@ -15,7 +15,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; -import java.util.stream.Stream; +import java.nio.file.DirectoryStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -142,20 +142,27 @@ public class TempFileCleanupServiceTest { // Use MockedStatic to mock Files operations try (MockedStatic mockedFiles = mockStatic(Files.class)) { - // Mock Files.list for each directory we'll process - mockedFiles.when(() -> Files.list(eq(systemTempDir))) - .thenReturn(Stream.of( - ourTempFile1, ourTempFile2, oldTempFile, sysTempFile1, - jettyFile1, jettyFile2, regularFile, emptyFile, nestedDir)); + // Mock Files.newDirectoryStream for each directory we'll process + mockedFiles.when(() -> Files.newDirectoryStream(eq(systemTempDir))) + .thenReturn(directoryStreamOf( + ourTempFile1, + ourTempFile2, + oldTempFile, + sysTempFile1, + jettyFile1, + jettyFile2, + regularFile, + emptyFile, + nestedDir)); - mockedFiles.when(() -> Files.list(eq(customTempDir))) - .thenReturn(Stream.of(ourTempFile3, ourTempFile4, sysTempFile2, sysTempFile3)); + mockedFiles.when(() -> Files.newDirectoryStream(eq(customTempDir))) + .thenReturn(directoryStreamOf(ourTempFile3, ourTempFile4, sysTempFile2, sysTempFile3)); - mockedFiles.when(() -> Files.list(eq(libreOfficeTempDir))) - .thenReturn(Stream.of(ourTempFile5)); + mockedFiles.when(() -> Files.newDirectoryStream(eq(libreOfficeTempDir))) + .thenReturn(directoryStreamOf(ourTempFile5)); - mockedFiles.when(() -> Files.list(eq(nestedDir))) - .thenReturn(Stream.of(nestedTempFile)); + mockedFiles.when(() -> Files.newDirectoryStream(eq(nestedDir))) + .thenReturn(directoryStreamOf(nestedTempFile)); // Configure Files.isDirectory for each path mockedFiles.when(() -> Files.isDirectory(eq(nestedDir))).thenReturn(true); @@ -251,9 +258,10 @@ public class TempFileCleanupServiceTest { // Use MockedStatic to mock Files operations try (MockedStatic mockedFiles = mockStatic(Files.class)) { - // Mock Files.list for systemTempDir - mockedFiles.when(() -> Files.list(eq(systemTempDir))) - .thenReturn(Stream.of(ourTempFile, sysTempFile, regularFile)); + // Mock Files.newDirectoryStream for systemTempDir + mockedFiles + .when(() -> Files.newDirectoryStream(eq(systemTempDir))) + .thenReturn(directoryStreamOf(ourTempFile, sysTempFile, regularFile)); // Configure Files.isDirectory mockedFiles.when(() -> Files.isDirectory(any(Path.class))).thenReturn(false); @@ -302,9 +310,10 @@ public class TempFileCleanupServiceTest { // Use MockedStatic to mock Files operations try (MockedStatic mockedFiles = mockStatic(Files.class)) { - // Mock Files.list for systemTempDir - mockedFiles.when(() -> Files.list(eq(systemTempDir))) - .thenReturn(Stream.of(emptyFile, recentEmptyFile)); + // Mock Files.newDirectoryStream for systemTempDir + mockedFiles + .when(() -> Files.newDirectoryStream(eq(systemTempDir))) + .thenReturn(directoryStreamOf(emptyFile, recentEmptyFile)); // Configure Files.isDirectory mockedFiles.when(() -> Files.isDirectory(any(Path.class))).thenReturn(false); @@ -369,18 +378,22 @@ public class TempFileCleanupServiceTest { // Use MockedStatic to mock Files operations try (MockedStatic mockedFiles = mockStatic(Files.class)) { - // Mock Files.list for each directory - mockedFiles.when(() -> Files.list(eq(systemTempDir))) - .thenReturn(Stream.of(dir1)); + // Mock Files.newDirectoryStream for each directory + mockedFiles + .when(() -> Files.newDirectoryStream(eq(systemTempDir))) + .thenReturn(directoryStreamOf(dir1)); - mockedFiles.when(() -> Files.list(eq(dir1))) - .thenReturn(Stream.of(tempFile1, dir2)); + mockedFiles + .when(() -> Files.newDirectoryStream(eq(dir1))) + .thenReturn(directoryStreamOf(tempFile1, dir2)); - mockedFiles.when(() -> Files.list(eq(dir2))) - .thenReturn(Stream.of(tempFile2, dir3)); + mockedFiles + .when(() -> Files.newDirectoryStream(eq(dir2))) + .thenReturn(directoryStreamOf(tempFile2, dir3)); - mockedFiles.when(() -> Files.list(eq(dir3))) - .thenReturn(Stream.of(tempFile3)); + mockedFiles + .when(() -> Files.newDirectoryStream(eq(dir3))) + .thenReturn(directoryStreamOf(tempFile3)); // Configure Files.isDirectory for each path mockedFiles.when(() -> Files.isDirectory(eq(dir1))).thenReturn(true); @@ -461,4 +474,16 @@ public class TempFileCleanupServiceTest { private static Path eq(Path path) { return argThat(arg -> arg != null && arg.equals(path)); } + + private static DirectoryStream directoryStreamOf(Path... paths) { + return new DirectoryStream<>() { + @Override + public java.util.Iterator iterator() { + return java.util.Arrays.asList(paths).iterator(); + } + + @Override + public void close() {} + }; + } } diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index a26f256f7..c156df9a7 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -134,6 +134,8 @@ system: cleanupIntervalMinutes: 30 # How often to run cleanup (in minutes) startupCleanup: true # Clean up old temp files on startup cleanupSystemTemp: false # Whether to clean broader system temp directory + batchSize: 0 # Number of entries processed before optional pause (0 = unlimited) + pauseBetweenBatchesMs: 0 # Pause duration in milliseconds between batches ui: appName: '' # application's visible name