This commit is contained in:
Anthony Stirling 2025-07-21 13:08:21 +01:00
parent fe4a452a54
commit 73a340cbe2
2 changed files with 137 additions and 114 deletions

View File

@ -23,53 +23,60 @@ public class CleanupAsyncConfig {
exec.setMaxPoolSize(1); exec.setMaxPoolSize(1);
exec.setQueueCapacity(100); exec.setQueueCapacity(100);
exec.setThreadNamePrefix("cleanup-"); exec.setThreadNamePrefix("cleanup-");
// Set custom rejection handler to log when queue is full // Set custom rejection handler to log when queue is full
exec.setRejectedExecutionHandler(new RejectedExecutionHandler() { exec.setRejectedExecutionHandler(
private volatile long lastRejectionTime = 0; new RejectedExecutionHandler() {
private volatile int rejectionCount = 0; private volatile long lastRejectionTime = 0;
private volatile int rejectionCount = 0;
@Override
public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { @Override
long currentTime = System.currentTimeMillis(); public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) {
rejectionCount++; long currentTime = System.currentTimeMillis();
rejectionCount++;
// Rate-limit logging to avoid spam
if (currentTime - lastRejectionTime > 60000) { // Log at most once per minute // Rate-limit logging to avoid spam
log.warn("Cleanup task rejected #{} - queue full! Active: {}, Queue size: {}, Pool size: {}", if (currentTime - lastRejectionTime
rejectionCount, > 60000) { // Log at most once per minute
executor.getActiveCount(), log.warn(
executor.getQueue().size(), "Cleanup task rejected #{} - queue full! Active: {}, Queue size: {}, Pool size: {}",
executor.getPoolSize()); rejectionCount,
lastRejectionTime = currentTime; executor.getActiveCount(),
} executor.getQueue().size(),
executor.getPoolSize());
// Try to discard oldest task and add this one lastRejectionTime = currentTime;
if (executor.getQueue().poll() != null) { }
log.debug("Discarded oldest queued cleanup task to make room");
try { // Try to discard oldest task and add this one
executor.execute(r); if (executor.getQueue().poll() != null) {
return; log.debug("Discarded oldest queued cleanup task to make room");
} catch (Exception e) { try {
// If still rejected, fall back to caller-runs 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);
}
} }
} });
// 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);
}
}
});
exec.initialize(); exec.initialize();
return exec; return exec;
} }

View File

@ -49,10 +49,10 @@ public class TempFileCleanupService {
// Maximum recursion depth for directory traversal // Maximum recursion depth for directory traversal
private static final int MAX_RECURSION_DEPTH = 5; private static final int MAX_RECURSION_DEPTH = 5;
// Maximum consecutive failures before aborting batch cleanup // Maximum consecutive failures before aborting batch cleanup
private static final int MAX_CONSECUTIVE_FAILURES = 10; private static final int MAX_CONSECUTIVE_FAILURES = 10;
// Cleanup state management // Cleanup state management
private final AtomicBoolean cleanupRunning = new AtomicBoolean(false); private final AtomicBoolean cleanupRunning = new AtomicBoolean(false);
private final AtomicLong lastCleanupDuration = new AtomicLong(0); private final AtomicLong lastCleanupDuration = new AtomicLong(0);
@ -142,46 +142,68 @@ public class TempFileCleanupService {
public CompletableFuture<Void> scheduledCleanup() { public CompletableFuture<Void> scheduledCleanup() {
// Check if cleanup is already running // Check if cleanup is already running
if (!cleanupRunning.compareAndSet(false, true)) { if (!cleanupRunning.compareAndSet(false, true)) {
log.warn("Cleanup already in progress (running for {}ms), skipping this cycle", log.warn(
System.currentTimeMillis() - lastCleanupTimestamp.get()); "Cleanup already in progress (running for {}ms), skipping this cycle",
System.currentTimeMillis() - lastCleanupTimestamp.get());
return CompletableFuture.completedFuture(null); return CompletableFuture.completedFuture(null);
} }
// Calculate timeout as 2x cleanup interval // Calculate timeout as 2x cleanup interval
long timeoutMinutes = applicationProperties.getSystem().getTempFileManagement().getCleanupIntervalMinutes() * 2; long timeoutMinutes =
applicationProperties
return CompletableFuture.supplyAsync(() -> { .getSystem()
long startTime = System.currentTimeMillis(); .getTempFileManagement()
lastCleanupTimestamp.set(startTime); .getCleanupIntervalMinutes()
long cleanupNumber = cleanupCount.incrementAndGet(); * 2;
try { CompletableFuture<Void> cleanupFuture =
log.info("Starting cleanup #{} with {}min timeout", cleanupNumber, timeoutMinutes); CompletableFuture.runAsync(
doScheduledCleanup(); () -> {
long startTime = System.currentTimeMillis();
long duration = System.currentTimeMillis() - startTime; lastCleanupTimestamp.set(startTime);
lastCleanupDuration.set(duration); long cleanupNumber = cleanupCount.incrementAndGet();
log.info("Cleanup #{} completed successfully in {}ms", cleanupNumber, duration);
return null; try {
} catch (Exception e) { log.info(
long duration = System.currentTimeMillis() - startTime; "Starting cleanup #{} with {}min timeout",
lastCleanupDuration.set(duration); cleanupNumber,
log.error("Cleanup #{} failed after {}ms", cleanupNumber, duration, e); timeoutMinutes);
return null; doScheduledCleanup();
} finally {
cleanupRunning.set(false); long duration = System.currentTimeMillis() - startTime;
} lastCleanupDuration.set(duration);
}).orTimeout(timeoutMinutes, TimeUnit.MINUTES) log.info(
.exceptionally(throwable -> { "Cleanup #{} completed successfully in {}ms",
if (throwable.getCause() instanceof TimeoutException) { cleanupNumber,
log.error("Cleanup #{} timed out after {}min - forcing cleanup state reset", duration);
cleanupCount.get(), timeoutMinutes); } catch (Exception e) {
cleanupRunning.set(false); long duration = System.currentTimeMillis() - startTime;
} lastCleanupDuration.set(duration);
return null; log.error(
}); "Cleanup #{} failed after {}ms",
cleanupNumber,
duration,
e);
} finally {
cleanupRunning.set(false);
}
});
return cleanupFuture
.orTimeout(timeoutMinutes, TimeUnit.MINUTES)
.exceptionally(
throwable -> {
if (throwable.getCause() instanceof TimeoutException) {
log.error(
"Cleanup #{} timed out after {}min - forcing cleanup state reset",
cleanupCount.get(),
timeoutMinutes);
cleanupRunning.set(false);
}
return null;
});
} }
/** Internal method that performs the actual cleanup work */ /** Internal method that performs the actual cleanup work */
private void doScheduledCleanup() { private void doScheduledCleanup() {
long maxAgeMillis = tempFileManager.getMaxAgeMillis(); long maxAgeMillis = tempFileManager.getMaxAgeMillis();
@ -407,10 +429,12 @@ public class TempFileCleanupService {
} else { } else {
log.warn("Failed to delete temp file: {}", path, e); log.warn("Failed to delete temp file: {}", path, e);
} }
if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) {
log.error("Aborting directory cleanup after {} consecutive failures in: {}", log.error(
consecutiveFailures, directory); "Aborting directory cleanup after {} consecutive failures in: {}",
consecutiveFailures,
directory);
return; // Early exit from cleanup return; // Early exit from cleanup
} }
} }
@ -418,10 +442,12 @@ public class TempFileCleanupService {
} catch (Exception e) { } catch (Exception e) {
consecutiveFailures++; consecutiveFailures++;
log.warn("Error processing path: {}", path, e); log.warn("Error processing path: {}", path, e);
if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) { if (consecutiveFailures >= MAX_CONSECUTIVE_FAILURES) {
log.error("Aborting directory cleanup after {} consecutive failures in: {}", log.error(
consecutiveFailures, directory); "Aborting directory cleanup after {} consecutive failures in: {}",
consecutiveFailures,
directory);
return; // Early exit from cleanup return; // Early exit from cleanup
} }
} }
@ -536,10 +562,8 @@ public class TempFileCleanupService {
log.warn("Failed to clean up PDFBox cache file", e); log.warn("Failed to clean up PDFBox cache file", e);
} }
} }
/** /** Get cleanup status and metrics for monitoring */
* Get cleanup status and metrics for monitoring
*/
public String getCleanupStatus() { public String getCleanupStatus() {
if (cleanupRunning.get()) { if (cleanupRunning.get()) {
long runningTime = System.currentTimeMillis() - lastCleanupTimestamp.get(); long runningTime = System.currentTimeMillis() - lastCleanupTimestamp.get();
@ -549,38 +573,30 @@ public class TempFileCleanupService {
long lastTime = lastCleanupTimestamp.get(); long lastTime = lastCleanupTimestamp.get();
if (lastTime > 0) { if (lastTime > 0) {
long timeSinceLastRun = System.currentTimeMillis() - lastTime; long timeSinceLastRun = System.currentTimeMillis() - lastTime;
return String.format("Last cleanup #%d: %dms duration, %dms ago", return String.format(
cleanupCount.get(), lastDuration, timeSinceLastRun); "Last cleanup #%d: %dms duration, %dms ago",
cleanupCount.get(), lastDuration, timeSinceLastRun);
} else { } else {
return "No cleanup runs yet"; return "No cleanup runs yet";
} }
} }
} }
/** /** Check if cleanup is currently running */
* Check if cleanup is currently running
*/
public boolean isCleanupRunning() { public boolean isCleanupRunning() {
return cleanupRunning.get(); return cleanupRunning.get();
} }
/** /** Get cleanup metrics */
* Get cleanup metrics
*/
public CleanupMetrics getMetrics() { public CleanupMetrics getMetrics() {
return new CleanupMetrics( return new CleanupMetrics(
cleanupCount.get(), cleanupCount.get(),
lastCleanupDuration.get(), lastCleanupDuration.get(),
lastCleanupTimestamp.get(), lastCleanupTimestamp.get(),
cleanupRunning.get() cleanupRunning.get());
);
} }
/** Simple record for cleanup metrics */ /** Simple record for cleanup metrics */
public record CleanupMetrics( public record CleanupMetrics(
long totalRuns, long totalRuns, long lastDurationMs, long lastRunTimestamp, boolean currentlyRunning) {}
long lastDurationMs,
long lastRunTimestamp,
boolean currentlyRunning
) {}
} }