diff --git a/container-core/src/main/java/com/yahoo/container/logging/AccessLogHandler.java b/container-core/src/main/java/com/yahoo/container/logging/AccessLogHandler.java index 16f93bf70729..c85b82ed19a6 100644 --- a/container-core/src/main/java/com/yahoo/container/logging/AccessLogHandler.java +++ b/container-core/src/main/java/com/yahoo/container/logging/AccessLogHandler.java @@ -13,7 +13,7 @@ class AccessLogHandler { AccessLogHandler(AccessLogConfig.FileHandler config, LogWriter logWriter) { logFileHandler = new LogFileHandler<>( toCompression(config), config.bufferSize(), config.pattern(), config.rotation(), - config.symlink(), queueSize(config), config.rotationSize(), "request-logger", logWriter); + config.symlink(), queueSize(config), "request-logger", logWriter); } private static int queueSize(AccessLogConfig.FileHandler config) { diff --git a/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java b/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java index a9e5d068ad09..97ba13739e52 100644 --- a/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java +++ b/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java @@ -19,9 +19,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; import java.util.ArrayList; import java.util.Optional; import java.util.concurrent.BlockingQueue; @@ -50,40 +47,18 @@ enum Compression {NONE, GZIP, ZSTD} @FunctionalInterface private interface Pollable { Operation poll() throws InterruptedException; } - LogFileHandler(Compression compression, int bufferSize, String filePattern, String rotationTimes, String symlinkName, - int queueSize, long rotationSize, String threadName, LogWriter logWriter, Clock clock) { - this(compression, bufferSize, filePattern, calcTimesMinutes(rotationTimes), symlinkName, queueSize, rotationSize, threadName, logWriter, clock); - } - - LogFileHandler(Compression compression, int bufferSize, String filePattern, long[] rotationTimes, String symlinkName, - int queueSize, long rotationSize, String threadName, LogWriter logWriter, Clock clock) { - this.logQueue = new LinkedBlockingQueue<>(queueSize); - this.logThread = new LogThread<>(logWriter, filePattern, compression, bufferSize, rotationTimes, symlinkName, rotationSize, threadName, this::poll, clock); - this.logThread.start(); - } - - LogFileHandler(Compression compression, int bufferSize, String filePattern, String rotationTimes, String symlinkName, - int queueSize, long rotationSize, String threadName, LogWriter logWriter) { - this(compression, bufferSize, filePattern, calcTimesMinutes(rotationTimes), symlinkName, queueSize, rotationSize, threadName, logWriter, Clock.systemUTC()); - } - - LogFileHandler(Compression compression, int bufferSize, String filePattern, long[] rotationTimes, String symlinkName, - int queueSize, long rotationSize, String threadName, LogWriter logWriter) { - this(compression, bufferSize, filePattern, rotationTimes, symlinkName, queueSize, rotationSize, threadName, logWriter, Clock.systemUTC()); - } - - // Keep backward compatibility constructors LogFileHandler(Compression compression, int bufferSize, String filePattern, String rotationTimes, String symlinkName, int queueSize, String threadName, LogWriter logWriter) { - this(compression, bufferSize, filePattern, calcTimesMinutes(rotationTimes), symlinkName, queueSize, 0, threadName, logWriter, Clock.systemUTC()); + this(compression, bufferSize, filePattern, calcTimesMinutes(rotationTimes), symlinkName, queueSize, threadName, logWriter); } LogFileHandler(Compression compression, int bufferSize, String filePattern, long[] rotationTimes, String symlinkName, int queueSize, String threadName, LogWriter logWriter) { - this(compression, bufferSize, filePattern, rotationTimes, symlinkName, queueSize, 0, threadName, logWriter, Clock.systemUTC()); + this.logQueue = new LinkedBlockingQueue<>(queueSize); + this.logThread = new LogThread<>(logWriter, filePattern, compression, bufferSize, rotationTimes, symlinkName, threadName, this::poll); + this.logThread.start(); } - private Operation poll() throws InterruptedException { return logQueue.poll(100, TimeUnit.MILLISECONDS); } @@ -212,10 +187,6 @@ static class LogThread extends Thread { long lastFlush = 0; private PageCacheFriendlyFileOutputStream fileOutput = null; private long nextRotationTime = 0; - private long fileSize = 0; - private final Duration fileSizeCheckInterval = Duration.ofMinutes(1); - private final Clock clock; - private Instant lastFileSizeCheck = Instant.now(); private final String filePattern; // default to current directory, ms time stamp private volatile String fileName; private final LogWriter logWriter; @@ -225,7 +196,7 @@ static class LogThread extends Thread { private final String symlinkName; private final ExecutorService executor = createCompressionTaskExecutor(); private final NativeIO nativeIO = new NativeIO(); - private final long rotationSize; + LogThread(LogWriter logWriter, String filePattern, @@ -233,10 +204,8 @@ static class LogThread extends Thread { int bufferSize, long[] rotationTimes, String symlinkName, - long rotationSize, String threadName, - Pollable operationProvider, - Clock clock) { + Pollable operationProvider) { super(threadName); setDaemon(true); this.logWriter = logWriter; @@ -245,22 +214,7 @@ static class LogThread extends Thread { this.bufferSize = bufferSize; this.rotationTimes = rotationTimes; this.symlinkName = (symlinkName != null && !symlinkName.isBlank()) ? symlinkName : null; - this.rotationSize = rotationSize; this.operationProvider = operationProvider; - this.clock = clock; - } - - LogThread(LogWriter logWriter, - String filePattern, - Compression compression, - int bufferSize, - long[] rotationTimes, - String symlinkName, - long rotationSize, - String threadName, - Pollable operationProvider) { - this(logWriter, filePattern, compression, bufferSize, rotationTimes, - symlinkName, rotationSize, threadName, operationProvider, Clock.systemUTC()); } private static ExecutorService createCompressionTaskExecutor() { @@ -340,18 +294,11 @@ private void internalPublish(LOGTYPE r) { // first check to see if new file needed. // if so, use this.internalRotateNow() to do it - long now = clock.millis(); - Instant nowInstant = Instant.ofEpochMilli(now); + long now = System.currentTimeMillis(); if (nextRotationTime <= 0) { nextRotationTime = getNextRotationTime(now); // lazy initialization } - if (lastFileSizeCheck.plus(fileSizeCheckInterval).isBefore(nowInstant)) { - getFileSize(nowInstant); - } - if (rotationSize > 0 && fileOutput != null && fileSize >= rotationSize) { - nextRotationTime = now; // trigger rotation based on size - } - if (now >= nextRotationTime || fileOutput == null) { + if (now > nextRotationTime || fileOutput == null) { internalRotateNow(); } try { @@ -362,17 +309,6 @@ private void internalPublish(LOGTYPE r) { } } - private void getFileSize(Instant now) { - if (fileOutput != null) { - try { - fileSize = Files.size(Paths.get(fileName)); - lastFileSizeCheck = now; - } catch (IOException e) { - logger.log(Level.WARNING, "Failed to get log file size: " + Exceptions.toMessageString(e), e); - } - } - } - /** * Find next rotation after specified time. * @@ -381,7 +317,7 @@ private void getFileSize(Instant now) { */ long getNextRotationTime(long now) { if (now <= 0) { - now = clock.millis(); + now = System.currentTimeMillis(); } long nowTod = timeOfDayMillis(now); long next = 0; @@ -416,7 +352,7 @@ private void internalRotateNow() { // figure out new file name, then String oldFileName = fileName; - long now = clock.millis(); + long now = System.currentTimeMillis(); fileName = LogFormatter.insertDate(filePattern, now); internalClose(); try { diff --git a/container-core/src/main/resources/configdefinitions/container.core.access-log.def b/container-core/src/main/resources/configdefinitions/container.core.access-log.def index 273d77dd1a7c..5fdb24f11290 100644 --- a/container-core/src/main/resources/configdefinitions/container.core.access-log.def +++ b/container-core/src/main/resources/configdefinitions/container.core.access-log.def @@ -24,6 +24,3 @@ fileHandler.queueSize int default=10000 # Buffer size for the output stream has a default of 256k fileHandler.bufferSize int default=262144 - -# Maximum file size (in bytes) before rotation. 0 means disabled (only time-based rotation) -fileHandler.rotationSize long default=0 diff --git a/container-core/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java b/container-core/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java index bd5f94689483..3cb877848ecc 100644 --- a/container-core/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java +++ b/container-core/src/test/java/com/yahoo/container/logging/LogFileHandlerTestCase.java @@ -4,7 +4,6 @@ import com.yahoo.compress.ZstdCompressor; import com.yahoo.container.logging.LogFileHandler.Compression; import com.yahoo.io.IOUtils; -import com.yahoo.test.ManualClock; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; @@ -24,15 +23,11 @@ import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.zip.GZIPInputStream; -import java.time.Duration; -import java.util.List; -import java.util.stream.Collectors; import static com.yahoo.yolean.Exceptions.uncheck; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -41,7 +36,6 @@ */ public class LogFileHandlerTestCase { private static final int BUFFER_SIZE = 0x10000; - private static final ManualClock clock = new ManualClock(); @TempDir public File temporaryFolder; @@ -212,240 +206,6 @@ private void testcompression(Compression compression, h.shutdown(); } - @Test - void testSizeBasedRotation() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testsizerotation"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - long rotationSize = 1024; - - LogFileHandler handler = new LogFileHandler<>( - Compression.NONE, BUFFER_SIZE, pattern, new long[]{0}, null, 2048, - rotationSize, "thread-name", new StringLogWriter(), clock); - - handler.publish("initial"); - handler.flush(); - Thread.sleep(100); - - String firstFile = handler.getFileName(); - assertNotNull(firstFile, "File should be created after first write"); - - String largeMessage = "x".repeat(100); - for (int i = 0; i < 15; i++) { - handler.publish(largeMessage); - } - handler.flush(); - - clock.advance(Duration.ofMinutes(1).plusSeconds(1)); - - handler.publish("trigger"); - handler.flush(); - Thread.sleep(200); - - String currentFile = handler.getFileName(); - assertNotEquals(firstFile, currentFile, "File should have rotated due to size"); - - assertTrue(Files.exists(Paths.get(firstFile)), "Original file should exist after rotation"); - assertTrue(Files.size(Paths.get(firstFile)) > 0, "Original file should have data"); - - handler.shutdown(); - } - - @Test - void testSizeRotationDisabled() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testsizerotationdisabled"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - LogFileHandler handler = new LogFileHandler<>( - Compression.NONE, BUFFER_SIZE, pattern, new long[]{0}, null, 2048, - 0, "thread-name", new StringLogWriter(), clock); - - handler.publish("initial"); - handler.flush(); - Thread.sleep(100); - - String firstFile = handler.getFileName(); - assertNotNull(firstFile, "File should be created after first write"); - - String largeMessage = "x".repeat(1000); - for (int i = 0; i < 100; i++) { - handler.publish(largeMessage); - } - handler.flush(); - - clock.advance(Duration.ofMinutes(2)); - handler.publish("test"); - handler.flush(); - Thread.sleep(100); - - assertEquals(firstFile, handler.getFileName(), "File should not rotate when size rotation is disabled"); - - handler.shutdown(); - } - - @Test - void testFileSizeCheckInterval() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testfilesizecheckinterval"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - long rotationSize = 1024; - - LogFileHandler handler = new LogFileHandler<>( - Compression.NONE, BUFFER_SIZE, pattern, new long[]{0}, null, 2048, - rotationSize, "thread-name", new StringLogWriter(), clock); - - handler.publish("initial"); - handler.flush(); - Thread.sleep(100); - - String firstFile = handler.getFileName(); - assertNotNull(firstFile); - - String message = "x".repeat(100); - for (int i = 0; i < 20; i++) { - handler.publish(message); - } - handler.flush(); - - clock.advance(Duration.ofSeconds(30)); - - handler.publish("test"); - handler.flush(); - Thread.sleep(100); - assertEquals(firstFile, handler.getFileName(), "File should not rotate before check interval"); - - clock.advance(Duration.ofSeconds(31)); - handler.publish("trigger"); - handler.flush(); - Thread.sleep(200); - - assertNotEquals(firstFile, handler.getFileName(), "File should rotate after size check"); - - handler.shutdown(); - } - - @Test - void testSizeAndTimeRotation() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testsizeandtimerotation"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - long rotationSize = 2048; - long[] rotationTimes = {clock.millis() + 3600000}; // 1 hour from now - - LogFileHandler handler = new LogFileHandler<>( - Compression.NONE, BUFFER_SIZE, pattern, rotationTimes, null, 2048, - rotationSize, "thread-name", new StringLogWriter(), clock); - - handler.publish("initial"); - handler.flush(); - Thread.sleep(100); - - String firstFile = handler.getFileName(); - assertNotNull(firstFile); - - String message = "x".repeat(100); - for (int i = 0; i < 10; i++) { - handler.publish(message); - } - handler.flush(); - Thread.sleep(100); - - assertEquals(firstFile, handler.getFileName(), "File should not rotate before size limit"); - - for (int i = 0; i < 15; i++) { - handler.publish(message); - } - handler.flush(); - - clock.advance(Duration.ofMinutes(1).plusSeconds(1)); - handler.publish("trigger"); - handler.flush(); - Thread.sleep(200); - - assertNotEquals(firstFile, handler.getFileName(), "File should rotate due to size limit"); - - handler.shutdown(); - } - - @Test - void testMultipleSizeRotations() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testmultiplesizerotations"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - long rotationSize = 512; - - LogFileHandler handler = new LogFileHandler<>( - Compression.NONE, BUFFER_SIZE, pattern, new long[]{0}, null, 2048, - rotationSize, "thread-name", new StringLogWriter(), clock); - - String message = "x".repeat(50); - - for (int rotation = 0; rotation < 3; rotation++) { - for (int i = 0; i < 12; i++) { - handler.publish(message); - } - handler.flush(); - - clock.advance(Duration.ofMinutes(1).plusSeconds(1)); - handler.publish("trigger" + rotation); - handler.flush(); - Thread.sleep(200); - } - - handler.shutdown(); - - List logFiles = Files.list(root.toPath()) - .filter(p -> p.toString().contains("logfilehandlertest")) - .collect(Collectors.toList()); - - assertTrue(logFiles.size() >= 3, "Should have created at least 3 log files due to size rotation"); - } - - @Test - void testSizeRotationWithCompression() throws IOException, InterruptedException { - File root = newFolder(temporaryFolder, "testsizerotationcompression"); - String pattern = root.getAbsolutePath() + "/logfilehandlertest.%Y%m%d%H%M%S%s"; - - LogFileHandler handler = new LogFileHandler<>( - Compression.ZSTD, BUFFER_SIZE, pattern, new long[]{0}, null, 2048, - 1024, "thread-name", new StringLogWriter(), clock); // Added clock parameter - - handler.publish("initial"); - handler.flush(); - Thread.sleep(100); - - String firstFile = handler.getFileName(); - assertNotNull(firstFile); - - String message = "x".repeat(100); - for (int i = 0; i < 20; i++) { - handler.publish(message); - } - handler.flush(); - - clock.advance(Duration.ofMinutes(2)); - handler.publish("trigger"); - handler.flush(); - Thread.sleep(200); - - String secondFile = handler.getFileName(); - assertNotEquals(firstFile, secondFile); - - handler.rotateNow(); - - int maxWaitTime = 5000; - int waited = 0; - while (Files.exists(Paths.get(firstFile)) && waited < maxWaitTime) { - Thread.sleep(100); - waited += 100; - } - - assertFalse(Files.exists(Paths.get(firstFile)), "Original file should be deleted after compression"); - assertTrue(Files.exists(Paths.get(firstFile + ".zst")), "Compressed file should exist"); - - handler.shutdown(); - } - static class StringLogWriter implements LogWriter { @Override