From fe4a452a54b46488836198f31f8991fdfd0b1d7a Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com.> Date: Mon, 21 Jul 2025 13:06:00 +0100 Subject: [PATCH] further cleanups --- .../common/config/CleanupAsyncConfig.java | 46 +++++++++++++++---- .../common/model/ApplicationProperties.java | 4 +- .../service/TempFileCleanupService.java | 19 ++++++++ .../src/main/resources/settings.yml.template | 4 +- 4 files changed, 61 insertions(+), 12 deletions(-) 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 index e5f39ae9e..b2d9c93ce 100644 --- a/app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java +++ b/app/common/src/main/java/stirling/software/common/config/CleanupAsyncConfig.java @@ -26,17 +26,47 @@ public class CleanupAsyncConfig { // Set custom rejection handler to log when queue is full exec.setRejectedExecutionHandler(new RejectedExecutionHandler() { + private volatile long lastRejectionTime = 0; + private volatile int rejectionCount = 0; + @Override public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { - log.warn("Cleanup task rejected - queue full! Active: {}, Queue size: {}, Pool size: {}", - executor.getActiveCount(), - executor.getQueue().size(), - executor.getPoolSize()); + long currentTime = System.currentTimeMillis(); + rejectionCount++; - // Use caller-runs policy as fallback - this will block the scheduler thread - // but ensures the cleanup still happens - log.warn("Executing cleanup task on scheduler thread as fallback"); - r.run(); + // Rate-limit logging to avoid spam + if (currentTime - lastRejectionTime > 60000) { // Log at most once per minute + log.warn("Cleanup task rejected #{} - queue full! Active: {}, Queue size: {}, Pool size: {}", + rejectionCount, + executor.getActiveCount(), + executor.getQueue().size(), + executor.getPoolSize()); + lastRejectionTime = currentTime; + } + + // Try to discard oldest task and add this one + if (executor.getQueue().poll() != null) { + log.debug("Discarded oldest queued cleanup task to make room"); + try { + executor.execute(r); + return; + } catch (Exception e) { + // If still rejected, fall back to caller-runs + } + } + + // Last resort: caller-runs with timeout protection + log.warn("Executing cleanup task #{} on scheduler thread as last resort", rejectionCount); + long startTime = System.currentTimeMillis(); + try { + r.run(); + long duration = System.currentTimeMillis() - startTime; + if (duration > 30000) { // Warn if cleanup blocks scheduler for >30s + log.warn("Cleanup task on scheduler thread took {}ms - consider tuning", duration); + } + } catch (Exception e) { + log.error("Cleanup task failed on scheduler thread", e); + } } }); 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 fec667e33..8ae5680fd 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,8 +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; + private int batchSize = 1000; + private long pauseBetweenBatchesMs = 50; 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 26179e4e8..027606b7b 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 @@ -50,6 +50,9 @@ public class TempFileCleanupService { // Maximum recursion depth for directory traversal private static final int MAX_RECURSION_DEPTH = 5; + // Maximum consecutive failures before aborting batch cleanup + private static final int MAX_CONSECUTIVE_FAILURES = 10; + // Cleanup state management private final AtomicBoolean cleanupRunning = new AtomicBoolean(false); private final AtomicLong lastCleanupDuration = new AtomicLong(0); @@ -371,6 +374,7 @@ public class TempFileCleanupService { .getTempFileManagement() .getPauseBetweenBatchesMs(); int processed = 0; + int consecutiveFailures = 0; try (java.nio.file.DirectoryStream stream = Files.newDirectoryStream(directory)) { for (Path path : stream) { @@ -394,17 +398,32 @@ public class TempFileCleanupService { try { Files.deleteIfExists(path); onDeleteCallback.accept(path); + consecutiveFailures = 0; // Reset failure count on success } catch (IOException e) { + consecutiveFailures++; 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 (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { + log.error("Aborting directory cleanup after {} consecutive failures in: {}", + consecutiveFailures, directory); + return; // Early exit from cleanup + } } } } catch (Exception e) { + consecutiveFailures++; log.warn("Error processing path: {}", path, e); + + if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { + log.error("Aborting directory cleanup after {} consecutive failures in: {}", + consecutiveFailures, directory); + return; // Early exit from cleanup + } } processed++; diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index c156df9a7..834868163 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -134,8 +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 + batchSize: 1000 # Number of entries processed before optional pause (0 = unlimited) + pauseBetweenBatchesMs: 50 # Pause duration in milliseconds between batches ui: appName: '' # application's visible name