From 066cb7984242442c45e20d327d752ccbc1728222 Mon Sep 17 00:00:00 2001
From: Andrea Selva <selva.andre@gmail.com>
Date: Thu, 20 Jun 2024 17:52:19 +0200
Subject: [PATCH] Avoid to log file not found errors when DLQ segments are
 removed concurrently between writer and reader. (#16204)

* Rework the logic to delete DLQ eldest segments to be more resilient on file not found errors and avoid to log warn messages that there isn't any action the user can do to solve.

* Fixed test case, when path point to a file that doesn't exist, rely always on path name comparator. Reworked the code to simplify, not needing anymore the tri-state variable

(cherry picked from commit 321e407e537c16ad32b69be844ebfc1f0aea02d5)
---
 .../common/io/DeadLetterQueueReader.java      | 35 ++++++++++++-------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/logstash-core/src/main/java/org/logstash/common/io/DeadLetterQueueReader.java b/logstash-core/src/main/java/org/logstash/common/io/DeadLetterQueueReader.java
index e76da789b42..b445406ed4b 100644
--- a/logstash-core/src/main/java/org/logstash/common/io/DeadLetterQueueReader.java
+++ b/logstash-core/src/main/java/org/logstash/common/io/DeadLetterQueueReader.java
@@ -374,24 +374,32 @@ private void removeSegmentsBefore(Path validSegment) throws IOException {
     }
 
     private int compareByFileTimestamp(Path p1, Path p2) {
-        FileTime timestamp1;
-        // if one of the getLastModifiedTime raise an error, consider them equals
-        // and fallback to the other comparator
-        try {
-            timestamp1 = Files.getLastModifiedTime(p1);
-        } catch (IOException ex) {
-            logger.warn("Error reading file's timestamp for {}", p1, ex);
+        final Optional<FileTime> timestampResult1 = readLastModifiedTime(p1);
+        final Optional<FileTime> timestampResult2 = readLastModifiedTime(p2);
+
+        // if one of the readLastModifiedTime encountered a file not found error or generic IO error,
+        // consider them equals and fallback to the other comparator
+        if (!timestampResult1.isPresent() || !timestampResult2.isPresent()) {
             return 0;
         }
 
-        FileTime timestamp2;
+        return timestampResult1.get().compareTo(timestampResult2.get());
+    }
+
+    /**
+     * Builder method. Read the timestamp if file on path p is present.
+     * When the file
+     * */
+    private static Optional<FileTime> readLastModifiedTime(Path p) {
         try {
-            timestamp2 = Files.getLastModifiedTime(p2);
+            return Optional.of(Files.getLastModifiedTime(p));
+        } catch (NoSuchFileException fileNotFoundEx) {
+            logger.debug("File {} doesn't exist", p);
+            return Optional.empty();
         } catch (IOException ex) {
-            logger.warn("Error reading file's timestamp for {}", p2, ex);
-            return 0;
+            logger.warn("Error reading file's timestamp for {}", p, ex);
+            return Optional.empty();
         }
-        return timestamp1.compareTo(timestamp2);
     }
 
     /**
@@ -407,6 +415,9 @@ private Optional<Long> deleteSegment(Path segment) {
             Files.delete(segment);
             logger.debug("Deleted segment {}", segment);
             return Optional.of(eventsInSegment);
+        } catch (NoSuchFileException fileNotFoundEx) {
+            logger.debug("Expected file segment {} was already removed by a writer", segment);
+            return Optional.empty();
         } catch (IOException ex) {
             logger.warn("Problem occurred in cleaning the segment {} after a repositioning", segment, ex);
             return Optional.empty();