Merge pull request #16248 from vespa-engine/bjorncs/access-log-cleanup

Bjorncs/access log cleanup

2 files changed, 17 insertions, 31 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/container/logging/LogFileHandler.java b/jdisc_http_service/src/main/java/com/yahoo/container/logging/LogFileHandler.java
index c3c7bb64dc7..104c75a7b88 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/container/logging/LogFileHandler.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/container/logging/LogFileHandler.java
@@ -6,7 +6,6 @@ import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.io.NativeIO;
 import com.yahoo.log.LogFileDb;
 import com.yahoo.protect.Process;
-import com.yahoo.system.ProcessExecuter;
 import com.yahoo.yolean.Exceptions;
 
 import java.io.BufferedOutputStream;
@@ -21,7 +20,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Optional;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.CountDownLatch;
@@ -43,7 +41,7 @@ class LogFileHandler <LOGTYPE> {
     enum Compression {NONE, GZIP, ZSTD}
 
     private final static Logger logger = Logger.getLogger(LogFileHandler.class.getName());
-    private final ArrayBlockingQueue<Operation<LOGTYPE>> logQueue = new ArrayBlockingQueue<>(100000);
+    private final ArrayBlockingQueue<Operation<LOGTYPE>> logQueue = new ArrayBlockingQueue<>(10000);
     final LogThread<LOGTYPE> logThread;
 
     @FunctionalInterface private interface Pollable<T> { Operation<T> poll() throws InterruptedException; }
@@ -75,6 +73,10 @@ class LogFileHandler <LOGTYPE> {
         addOperation(new Operation<>(r));
     }
 
+    void publishAndWait(LOGTYPE r) {
+        addOperationAndWait(new Operation<>(r));
+    }
+
     public void flush() {
         addOperationAndWait(new Operation<>(Operation.Type.flush));
     }
@@ -439,18 +441,13 @@ class LogFileHandler <LOGTYPE> {
          */
         private void createSymlinkToCurrentFile() {
             if (symlinkName == null) return;
-            File f = new File(fileName);
-            File f2 = new File(f.getParent(), symlinkName);
-            String[] cmd = new String[]{"/bin/ln", "-sf", f.getName(), f2.getPath()};
+            Path target = Paths.get(fileName);
+            Path link = target.resolveSibling(symlinkName);
             try {
-                int retval = new ProcessExecuter().exec(cmd).getFirst();
-                // Detonator pattern: Think of all the fun we can have if ln isn't what we
-                // think it is, if it doesn't return, etc, etc
-                if (retval != 0) {
-                    logger.warning("Command '" + Arrays.toString(cmd) + "' + failed with exitcode=" + retval);
-                }
+                Files.deleteIfExists(link);
+                Files.createSymbolicLink(link, target.getFileName());
             } catch (IOException e) {
-                logger.warning("Got '" + e + "' while doing'" + Arrays.toString(cmd) + "'.");
+                logger.log(Level.WARNING, "Failed to create symbolic link to current log file: " + e.getMessage(), e);
             }
         }
 
diff --git a/jdisc_http_service/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java
index cd3c174a12e..013f42cd5b0 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java
@@ -26,6 +26,7 @@ import java.util.zip.GZIPInputStream;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertNotEquals;
 
 /**
  * @author Bob Travis
@@ -106,43 +107,31 @@ public class LogFileHandlerTestCase {
                 Compression.NONE, root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s", new long[]{0}, "symlink", new StringLogWriter());
 
         String message = formatter.format(new LogRecord(Level.INFO, "test"));
-        handler.publish(message);
-        String firstFile;
-        do {
-             Thread.sleep(1);
-             firstFile = handler.getFileName();
-        } while (firstFile == null);
+        handler.publishAndWait(message);
+        String firstFile = handler.getFileName();
         handler.rotateNow();
-        String secondFileName;
-        do {
-            Thread.sleep(1);
-            secondFileName = handler.getFileName();
-        } while (firstFile.equals(secondFileName));
+        String secondFileName = handler.getFileName();
+        assertNotEquals(firstFile, secondFileName);
 
         String longMessage = formatter.format(new LogRecord(Level.INFO, "string which is way longer than the word test"));
         handler.publish(longMessage);
         handler.flush();
         assertThat(Files.size(Paths.get(firstFile))).isEqualTo(31);
         final long expectedSecondFileLength = 72;
-        long secondFileLength;
-        do {
-            Thread.sleep(1);
-            secondFileLength = Files.size(Paths.get(secondFileName));
-        } while (secondFileLength != expectedSecondFileLength);
 
         long symlinkFileLength = Files.size(root.toPath().resolve("symlink"));
         assertThat(symlinkFileLength).isEqualTo(expectedSecondFileLength);
         handler.shutdown();
     }
 
-    @Test
+    @Test(timeout = /*5 minutes*/300_000)
     public void testcompression_gzip() throws InterruptedException, IOException {
         testcompression(
                 Compression.GZIP, "gz",
                 (compressedFile, __) -> uncheck(() -> new String(new GZIPInputStream(Files.newInputStream(compressedFile)).readAllBytes())));
     }
 
-    @Test
+    @Test(timeout = /*5 minutes*/300_000)
     public void testcompression_zstd() throws InterruptedException, IOException {
         testcompression(
                 Compression.ZSTD, "zst",