Keep last 1000 lines sorted in log reader

2 files changed, 36 insertions, 16 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/LogReader.java b/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
index fe841bac68e..9f270acce5f 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/LogReader.java
@@ -29,6 +29,7 @@ import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
+import java.util.PriorityQueue;
 import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -73,10 +74,18 @@ class LogReader {
 
                 Iterator<LineWithTimestamp> lines = Iterators.mergeSorted(logLineIterators,
                                                                           Comparator.comparingDouble(LineWithTimestamp::timestamp));
+                PriorityQueue<LineWithTimestamp> heap = new PriorityQueue<>(Comparator.comparingDouble(LineWithTimestamp::timestamp));
                 while (lines.hasNext()) {
+                    heap.offer(lines.next());
+                    if (heap.size() > 1000) {
+                        if (linesWritten++ >= maxLines) return;
+                        writer.write(heap.poll().line);
+                        writer.newLine();
+                    }
+                }
+                while ( ! heap.isEmpty()) {
                     if (linesWritten++ >= maxLines) return;
-                    String line = lines.next().line();
-                    writer.write(line);
+                    writer.write(heap.poll().line);
                     writer.newLine();
                 }
             }
@@ -170,7 +179,7 @@ class LogReader {
                     if (parts.length != 7)
                         continue;
 
-                    if (hostname.map(host -> !host.equals(parts[1])).orElse(false))
+                    if (hostname.map(host -> ! host.equals(parts[1])).orElse(false))
                         continue;
 
                     double timestamp = Double.parseDouble(parts[0]);
diff --git a/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java b/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
index e98f8cac276..749033cd1bf 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/LogReaderTest.java
@@ -3,8 +3,8 @@ package com.yahoo.container.handler;
 
 import com.yahoo.compress.ZstdCompressor;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIf;
 import org.junit.jupiter.api.io.TempDir;
 
 import java.io.ByteArrayOutputStream;
@@ -30,7 +30,8 @@ public class LogReaderTest {
 
     private static final String logv11 = "3600.2\tnode1.com\t5480\tcontainer\tstdout\tinfo\tfourth\n";
     private static final String logv   = "90000.1\tnode1.com\t5480\tcontainer\tstdout\tinfo\tlast\n";
-    private static final String log100 = "0.2\tnode2.com\t5480\tcontainer\tstdout\tinfo\tsecond\n";
+    private static final String log100a = "0.2\tnode2.com\t5480\tcontainer\tstdout\tinfo\tsecond\n";
+    private static final String log100b = "0.15\tnode2.com\t5480\tcontainer\tstdout\tinfo\tfirst\n";
     private static final String log101 = "0.1\tnode2.com\t5480\tcontainer\tstdout\tinfo\tERROR: Bundle canary-application [71] Unable to get module class path. (java.lang.NullPointerException)\n";
     private static final String log110 = "3600.1\tnode1.com\t5480\tcontainer\tstderr\twarning\tthird\n";
     private static final String log200 = "86400.1\tnode2.com\t5480\tcontainer\tstderr\twarning\tjava.lang.NullPointerException\\n\\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\\n\\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)\n";
@@ -41,27 +42,37 @@ public class LogReaderTest {
         // Log archive paths and file names indicate what hour they contain logs for, with the start of that hour.
         // Multiple entries may exist for each hour.
         Files.createDirectories(logDirectory.resolve("1970/01/01"));
-        Files.write(logDirectory.resolve("1970/01/01/00-0.gz"), compress1(log100));
-        Files.write(logDirectory.resolve("1970/01/01/00-1"), log101.getBytes(UTF_8));
+        // Files may contain out-of-order entries.
+        Files.write(logDirectory.resolve("1970/01/01/00-0.gz"), compress1(log100a + log100b));
+        Files.writeString(logDirectory.resolve("1970/01/01/00-1"), log101);
         Files.write(logDirectory.resolve("1970/01/01/01-0.zst"), compress2(log110));
 
         Files.createDirectories(logDirectory.resolve("1970/01/02"));
-        Files.write(logDirectory.resolve("1970/01/02/00-0"), log200.getBytes(UTF_8));
+        Files.writeString(logDirectory.resolve("1970/01/02/00-0"), log200);
 
         // Vespa log file names are the second-truncated timestamp of the last entry.
         // The current log file has no timestamp suffix.
-        Files.write(logDirectory.resolve("vespa.log-1970-01-01.01-00-00"), logv11.getBytes(UTF_8));
-        Files.write(logDirectory.resolve("vespa.log"), logv.getBytes(UTF_8));
+        Files.writeString(logDirectory.resolve("vespa.log-1970-01-01.01-00-00"), logv11);
+        Files.writeString(logDirectory.resolve("vespa.log"), logv);
     }
 
-    @Disabled
+    private static boolean hasZstdcat() {
+        try {
+            return new ProcessBuilder("zstdcat", "--version").start().waitFor() == 0;
+        }
+        catch (Exception e) {
+            return false;
+        }
+    }
+
+    @EnabledIf("hasZstdcat")
     @Test
     void testThatLogsOutsideRangeAreExcluded() {
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
         logReader.writeLogs(baos, Instant.ofEpochMilli(150), Instant.ofEpochMilli(3601050), 100, Optional.empty());
 
-        assertEquals(log100 + logv11 + log110, baos.toString(UTF_8));
+        assertEquals(log100b + log100a + logv11 + log110, baos.toString(UTF_8));
     }
 
     @Test
@@ -73,14 +84,14 @@ public class LogReaderTest {
         assertEquals(log101 + logv11, baos.toString(UTF_8));
     }
 
-    @Disabled // TODO: zts log line missing on Mac
+    @EnabledIf("hasZstdcat")
     @Test
     void testZippedStreaming() {
         ByteArrayOutputStream zippedBaos = new ByteArrayOutputStream();
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
         logReader.writeLogs(zippedBaos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), 100, Optional.empty());
 
-        assertEquals(log101 + log100 + logv11 + log110 + log200 + logv, zippedBaos.toString(UTF_8));
+        assertEquals(log101 + log100b + log100a + logv11 + log110 + log200 + logv, zippedBaos.toString(UTF_8));
     }
 
     @Test
@@ -89,7 +100,7 @@ public class LogReaderTest {
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
         logReader.writeLogs(baos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), 100, Optional.of("node2.com"));
 
-        assertEquals(log101 + log100 + log200, baos.toString(UTF_8));
+        assertEquals(log101 + log100b + log100a + log200, baos.toString(UTF_8));
     }
 
     @Test
@@ -98,7 +109,7 @@ public class LogReaderTest {
         LogReader logReader = new LogReader(logDirectory, Pattern.compile(".*"));
         logReader.writeLogs(baos, Instant.EPOCH, Instant.EPOCH.plus(Duration.ofDays(2)), 2, Optional.of("node2.com"));
 
-        assertEquals(log101 + log100, baos.toString(UTF_8));
+        assertEquals(log101 + log100b, baos.toString(UTF_8));
     }
 
     private byte[] compress1(String input) throws IOException {