Copilot commented on code in PR #10821:
URL: https://github.com/apache/ozone/pull/10821#discussion_r3710692917
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -1161,6 +1162,46 @@ private synchronized void
removeKeyFromCompactionLogTable(
}
}
+ /**
+ * Removes SST files from the backup directory that were hard-linked during
+ * {@code onCompactionBegin} but never recorded in the compaction log because
+ * the compaction did not complete (for example, due to an OM crash/restart).
+ */
+ @VisibleForTesting
+ void cleanupOrphanedSstBackupFiles() {
+ Path sstBackupDirPath = Paths.get(sstBackupDir);
+ if (!Files.isDirectory(sstBackupDirPath)) {
+ return;
+ }
+
+ Set<String> referencedFiles = compactionDag.getCompactionMap().keySet();
+ Set<String> orphanedFiles = new HashSet<>();
+ try (Stream<Path> pathStream = Files.list(sstBackupDirPath)) {
+ pathStream.filter(path -> path.getFileName().toString().toLowerCase()
+ .endsWith(SST_FILE_EXTENSION))
+ .forEach(path -> {
+ String fileName =
FilenameUtils.getBaseName(path.getFileName().toString());
+ if (!referencedFiles.contains(fileName)) {
+ orphanedFiles.add(fileName);
+ }
+ });
+ } catch (IOException e) {
+ LOG.warn("Failed to list SST backup directory " + sstBackupDir, e);
+ }
+
+ if (orphanedFiles.isEmpty()) {
+ return;
+ }
+
+ LOG.info("Removing orphaned SST backup files left by incomplete
compactions: {}",
+ orphanedFiles);
+ try (UncheckedAutoCloseable ignored =
getBootstrapStateLock().acquireReadLock()) {
+ removeSstFiles(orphanedFiles);
+ } catch (InterruptedException e) {
+ LOG.warn("Failed to remove orphaned SST backup files", e);
+ }
Review Comment:
The `InterruptedException` is currently swallowed, which clears the thread
interrupt flag and can make shutdown/stop handling unreliable. Restore the
interrupt status (and return) after logging.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -1161,6 +1162,46 @@ private synchronized void
removeKeyFromCompactionLogTable(
}
}
+ /**
+ * Removes SST files from the backup directory that were hard-linked during
+ * {@code onCompactionBegin} but never recorded in the compaction log because
+ * the compaction did not complete (for example, due to an OM crash/restart).
+ */
+ @VisibleForTesting
+ void cleanupOrphanedSstBackupFiles() {
+ Path sstBackupDirPath = Paths.get(sstBackupDir);
+ if (!Files.isDirectory(sstBackupDirPath)) {
+ return;
+ }
+
+ Set<String> referencedFiles = compactionDag.getCompactionMap().keySet();
+ Set<String> orphanedFiles = new HashSet<>();
+ try (Stream<Path> pathStream = Files.list(sstBackupDirPath)) {
+ pathStream.filter(path -> path.getFileName().toString().toLowerCase()
+ .endsWith(SST_FILE_EXTENSION))
+ .forEach(path -> {
Review Comment:
`toLowerCase()` without an explicit locale can behave incorrectly under
certain default locales (eg Turkish) when checking file extensions. Use
`Locale.ROOT` (or avoid lowercasing) for a locale-stable suffix check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
