From d7b14d68d5cdc0ddd792af5441dbc4e3149e976e Mon Sep 17 00:00:00 2001 From: Michael Dombrowski Date: Thu, 16 Nov 2023 10:26:15 -0500 Subject: [PATCH] fix: delete log even if nothing was uploaded --- .../greengrass/logmanager/LogManagerService.java | 16 ++++++++-------- .../logmanager/model/LogFileGroup.java | 2 ++ .../services/DiskSpaceManagementService.java | 8 +++++++- .../greengrass/features/log-manager-1.feature | 13 ++++++++++++- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/aws/greengrass/logmanager/LogManagerService.java b/src/main/java/com/aws/greengrass/logmanager/LogManagerService.java index 372da17d..b2aaa42b 100644 --- a/src/main/java/com/aws/greengrass/logmanager/LogManagerService.java +++ b/src/main/java/com/aws/greengrass/logmanager/LogManagerService.java @@ -506,14 +506,6 @@ private void handleCloudWatchAttemptStatus(CloudWatchAttempt cloudWatchAttempt) return; } - ProcessingFiles processingFiles = processingFilesInformation.get(componentName); - - // Delete files based on diskspace - List deletedHashes = - this.diskSpaceManagementService.freeDiskSpace(attemptLogInformation.getLogFileGroup()); - - processingFiles.remove(deletedHashes); // Stop tracking files already uploaded - // Delete after upload ComponentLogConfiguration componentLogConfiguration = componentLogConfigurations.get(componentName); completedFiles.forEach(file -> this.deleteFile(componentLogConfiguration, file)); @@ -706,6 +698,14 @@ private void processLogsAndUpload() throws InterruptedException { return; } + ProcessingFiles componentProcessingFiles = processingFilesInformation.get(componentName); + // Delete files based on diskspace + List deletedHashes = + this.diskSpaceManagementService.freeDiskSpace(logFileGroup); + if (componentProcessingFiles != null) { + componentProcessingFiles.remove(deletedHashes); // Stop tracking files that are deleted + } + if (logFileGroup.getLogFiles().isEmpty()) { continue; } diff --git a/src/main/java/com/aws/greengrass/logmanager/model/LogFileGroup.java b/src/main/java/com/aws/greengrass/logmanager/model/LogFileGroup.java index 2e87160f..4ec42d51 100644 --- a/src/main/java/com/aws/greengrass/logmanager/model/LogFileGroup.java +++ b/src/main/java/com/aws/greengrass/logmanager/model/LogFileGroup.java @@ -46,6 +46,7 @@ public List getProcessedLogFiles() { // Greater than or equal comparison means lastProcessed is afterOrEqual to the file.lastModified .filter(file -> this.lastProcessed.compareTo(Instant.ofEpochMilli(file.lastModified())) >= 0) .filter(file -> !this.isActiveFile(file)) + .sorted(Comparator.comparingLong(LogFile::lastModified)) .collect(Collectors.toList()); } @@ -55,6 +56,7 @@ public List getProcessedLogFiles() { public List getLogFiles() { return this.logFiles.stream() .filter(file -> this.lastProcessed.isBefore(Instant.ofEpochMilli(file.lastModified()))) + .sorted(Comparator.comparingLong(LogFile::lastModified)) .collect(Collectors.toList()); } diff --git a/src/main/java/com/aws/greengrass/logmanager/services/DiskSpaceManagementService.java b/src/main/java/com/aws/greengrass/logmanager/services/DiskSpaceManagementService.java index 8e7ac52f..e99da456 100644 --- a/src/main/java/com/aws/greengrass/logmanager/services/DiskSpaceManagementService.java +++ b/src/main/java/com/aws/greengrass/logmanager/services/DiskSpaceManagementService.java @@ -31,7 +31,13 @@ public List freeDiskSpace(LogFileGroup group) { long bytesDeleted = 0; long minimumBytesToBeDeleted = Math.max(group.totalSizeInBytes() - group.getMaxBytes().get(), 0); + // Prefer to delete files we've already processed, however we do also need to clean up files that we + // have not processed as they may exceed the max disk space setting. List deletableFiles = group.getProcessedLogFiles(); + deletableFiles.addAll(group.getLogFiles()); + // Do not delete active log file + deletableFiles.removeIf(group::isActiveFile); + List deletedHashes = new ArrayList<>(); for (LogFile logFile: deletableFiles) { @@ -48,4 +54,4 @@ public List freeDiskSpace(LogFileGroup group) { return deletedHashes; } -} \ No newline at end of file +} diff --git a/uat/testing-features/src/main/resources/greengrass/features/log-manager-1.feature b/uat/testing-features/src/main/resources/greengrass/features/log-manager-1.feature index bc7597fc..66a45d40 100644 --- a/uat/testing-features/src/main/resources/greengrass/features/log-manager-1.feature +++ b/uat/testing-features/src/main/resources/greengrass/features/log-manager-1.feature @@ -152,6 +152,7 @@ Feature: Greengrass V2 LogManager Scenario: LogManager-1-T3: As a customer I can configure the logs uploader to delete log oldest log files to keep the disk space limit specified on the configuration Given 15 temporary rotated log files for component UserComponentB have been created + And 15 temporary rotated log files for component UserComponentC have been created And 5 temporary rotated log files for component aws.greengrass.Nucleus have been created And 5 temporary rotated log files for component UserComponentA have been created Given I create a Greengrass deployment with components @@ -166,6 +167,15 @@ Feature: Greengrass V2 LogManager "UserComponentB": { "logFileRegex": "UserComponentB_(.)+.log", "logFileDirectoryPath": "${UserComponentBLogDirectory}", + "diskSpaceLimit":"0", + "diskSpaceLimitUnit":"KB", + "minimumLogLevel": "WARN", + "uploadToCloudWatch": "true", + "deleteLogFileAfterCloudUpload": "false" + }, + "UserComponentC": { + "logFileRegex": "UserComponentC_(.)+.log", + "logFileDirectoryPath": "${UserComponentCLogDirectory}", "diskSpaceLimit":"100", "diskSpaceLimitUnit":"KB", "minimumLogLevel": "INFO", @@ -189,7 +199,8 @@ Feature: Greengrass V2 LogManager Then the Greengrass deployment is COMPLETED on the device after 3 minutes Then I verify the aws.greengrass.LogManager component is RUNNING using the greengrass-cli And I wait 5 seconds - Then I verify that 15 log files for component UserComponentB are still available + Then I verify that 1 log files for component UserComponentB are still available + And I verify that 9 log files for component UserComponentC are still available @network Scenario: LogManager-1-T4: As a developer, logs uploader will handle network interruptions gracefully and upload logs from the last uploaded log after network resumes