further work on counter-based file size computation instead of period…

…ically calling file.length() Signed-off-by: Ceki Gulcu <[email protected]>
qos-ch · Aug 30, 2024 · d1172d0 · d1172d0
1 parent 466edb7
commit d1172d0
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 21 deletions.
diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java
@@ -48,8 +48,10 @@
 import org.slf4j.event.KeyValuePair;
 import org.slf4j.spi.MDCAdapter;
 
+import java.io.Console;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.charset.Charset;
 import java.text.SimpleDateFormat;
 import java.util.Date;
 
@@ -695,9 +697,16 @@ public void consoleCharsetTest() throws JoranException {
         if (EnvUtil.isJDK21OrHigher()) {
             configure(ClassicTestConstants.JORAN_INPUT_PREFIX + "consoleCharset.xml");
             checker.assertContainsMatch(Status.INFO, "About to instantiate property definer of type \\[ch.qos.logback.core.property.ConsoleCharsetPropertyDefiner\\]");
-            checker.assertContainsMatch(Status.WARN, "System.console\\(\\) returned null. Cannot compute console's charset, returning");
-            checker.assertContainsMatch("Setting property consoleCharset=null in scope LOCAL");
-            checker.assertContainsMatch("Converting the string \\\"null. as Charset.defaultCharset\\(\\)");
+
+            Console console = System.console();
+            if(console == null) {
+                checker.assertContainsMatch(Status.WARN, "System.console\\(\\) returned null. Cannot compute console's charset, returning");
+            } else {
+
+                boolean nullCharset =  checker.containsMatch("System.console() returned null charset. Returning \"NULL\" string as defined value.");
+                boolean foundCharset = checker.containsMatch("Found value '.*' as returned by System.console().");
+
+            }
             //StatusPrinter.print(loggerContext);
         }
     }

diff --git a/logback-core/src/main/java/ch/qos/logback/core/OutputStreamAppender.java b/logback-core/src/main/java/ch/qos/logback/core/OutputStreamAppender.java
@@ -21,6 +21,7 @@
 
 import ch.qos.logback.core.encoder.Encoder;
 import ch.qos.logback.core.encoder.LayoutWrappingEncoder;
+import ch.qos.logback.core.rolling.LengthCounter;
 import ch.qos.logback.core.spi.DeferredProcessingAware;
 import ch.qos.logback.core.status.ErrorStatus;
 
@@ -205,12 +206,16 @@ private void writeBytes(byte[] byteArray) throws IOException {
         try {
             if(isStarted()) {
                 writeByteArrayToOutputStreamWithPossibleFlush(byteArray);
+                updateByteCount(byteArray);
             }
         } finally {
             streamWriteLock.unlock();
         }
     }
 
+    protected void updateByteCount(byte[] byteArray) {
+    }
+
     /**
      * A simple method to write to an outputStream and flush the stream if immediateFlush is set to true.
      *

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/RollingFileAppender.java
@@ -130,9 +130,8 @@ private boolean checkForCollisionsInPreviousRollingFileAppenders() {
 
     private boolean innerCheckForFileNamePatternCollisionInPreviousRFA(FileNamePattern fileNamePattern) {
         boolean collisionsDetected = false;
-        @SuppressWarnings("unchecked")
-        Map<String, FileNamePattern> map = (Map<String, FileNamePattern>) context.getObject(
-                CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP);
+        @SuppressWarnings("unchecked") Map<String, FileNamePattern> map = (Map<String, FileNamePattern>) context.getObject(
+                        CoreConstants.RFA_FILENAME_PATTERN_COLLISION_MAP);
         if (map == null) {
             return collisionsDetected;
         }
@@ -270,7 +269,7 @@ public void setRollingPolicy(RollingPolicy policy) {
             String className = rollingPolicy.getClass().getSimpleName();
             addWarn("A rolling policy of type " + className + " was already set.");
             addWarn("Note that " + className + " doubles as a TriggeringPolicy");
-            addWarn("See also "+RFA_RESET_RP_OR_TP);
+            addWarn("See also " + RFA_RESET_RP_OR_TP);
         }
         this.rollingPolicy = policy;
         if (this.rollingPolicy instanceof TriggeringPolicy) {
@@ -284,11 +283,28 @@ public void setTriggeringPolicy(TriggeringPolicy<E> policy) {
             String className = triggeringPolicy.getClass().getSimpleName();
             addWarn("A triggering policy of type " + className + " was already set.");
             addWarn("Note that " + className + " doubles as a RollingPolicy");
-            addWarn("See also "+RFA_RESET_RP_OR_TP);
+            addWarn("See also " + RFA_RESET_RP_OR_TP);
         }
         triggeringPolicy = policy;
         if (policy instanceof RollingPolicy) {
             rollingPolicy = (RollingPolicy) policy;
         }
     }
+
+    @Override
+    protected void updateByteCount(byte[] byteArray) {
+
+        LengthCounter lengthCounter = getLengthCounter();
+        if (lengthCounter == null)
+            return;
+
+        if (byteArray != null && byteArray.length > 0) {
+            lengthCounter.add(byteArray.length);
+        }
+    }
+
+    private LengthCounter getLengthCounter() {
+        return triggeringPolicy.getLengthCounter();
+    }
+
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/TimeBasedRollingPolicy.java
@@ -238,6 +238,11 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
         return timeBasedFileNamingAndTriggeringPolicy.isTriggeringEvent(activeFile, event);
     }
 
+    @Override
+    public LengthCounter getLengthCounter() {
+        return timeBasedFileNamingAndTriggeringPolicy.getLengthCounter();
+    }
+
     /**
      * Get the number of archive files to keep.
      * 

diff --git a/...core/src/main/java/ch/qos/logback/core/rolling/helper/SizeAndTimeBasedArchiveRemover.java b/...core/src/main/java/ch/qos/logback/core/rolling/helper/SizeAndTimeBasedArchiveRemover.java
@@ -1,6 +1,6 @@
 /**
  * Logback: the reliable, generic, fast and flexible logging framework.
- * Copyright (C) 1999-2015, QOS.ch. All rights reserved.
+ * Copyright (C) 1999-2024, QOS.ch. All rights reserved.
  *
  * This program and the accompanying materials are dual-licensed under
  * either the terms of the Eclipse Public License v1.0 as published by
@@ -29,6 +29,7 @@ public SizeAndTimeBasedArchiveRemover(FileNamePattern fileNamePattern, RollingCa
         super(fileNamePattern, rc);
     }
 
+    @Override
     protected File[] getFilesInPeriod(Instant instantOfPeriodToClean) {
         File archive0 = new File(fileNamePattern.convertMultipleArguments(instantOfPeriodToClean, 0));
         File parentDir = getParentDir(archive0);
@@ -37,11 +38,6 @@ protected File[] getFilesInPeriod(Instant instantOfPeriodToClean) {
         return matchingFileArray;
     }
 
-    private String createStemRegex(final Instant instantOfPeriodToClean) {
-        String regex = fileNamePattern.toRegexForFixedDate(instantOfPeriodToClean);
-        return FileFilterUtil.afterLastSlash(regex);
-    }
-
     @Override
     protected void descendingSort(File[] matchingFileArray, Instant instant) {
 
@@ -79,4 +75,9 @@ private int extractIndex(Pattern pattern, File f1) {
         });
     }
 
+    private String createStemRegex(final Instant instantOfPeriodToClean) {
+        String regex = fileNamePattern.toRegexForFixedDate(instantOfPeriodToClean);
+        return FileFilterUtil.afterLastSlash(regex);
+    }
+
 }
diff --git a/...re/src/test/java/ch/qos/logback/core/rolling/TimeBasedRollingWithArchiveRemoval_Test.java b/...re/src/test/java/ch/qos/logback/core/rolling/TimeBasedRollingWithArchiveRemoval_Test.java
@@ -30,10 +30,12 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.LongAdder;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.time.temporal.ChronoUnit;
 
+import ch.qos.logback.core.util.StatusPrinter;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
@@ -265,7 +267,7 @@ public void dailyRolloverWithCronologPattern() {
     @Test
     public void dailySizeBasedRolloverWithoutCap() {
         SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object> sizeAndTimeBasedFNATP = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object>();
-        sizeAndTimeBasedFNATP.invocationGate = fixedRateInvocationGate;
+        //sizeAndTimeBasedFNATP.invocationGate = fixedRateInvocationGate;
 
         sizeAndTimeBasedFNATP.setMaxFileSize(new FileSize(10000));
         tbfnatp = sizeAndTimeBasedFNATP;
@@ -279,9 +281,8 @@ public void dailySizeBasedRolloverWithoutCap() {
     @Test
     public void dailySizeBasedRolloverWithSizeCap() {
         SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object> sizeAndTimeBasedFNATP = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object>();
-        sizeAndTimeBasedFNATP.invocationGate = new FixedRateInvocationGate(ticksPerPeriod / 8);
-        long bytesPerPeriod = 17000;
-        long fileSize = (bytesPerPeriod) / 5;
+
+        long fileSize = 3400;
         int expectedFileCount = 10;
         long sizeCap = expectedFileCount * fileSize;
         sizeAndTimeBasedFNATP.setMaxFileSize(new FileSize(fileSize));
@@ -303,14 +304,23 @@ public int compare(File f0, File f1) {
                 return s0.compareTo(s1);
             }
         });
-        checkFileCount(expectedFileCount - 1);
+
+        //StatusPrinter.print(context);
+        //foundFiles.forEach(f -> System.out.println(""+f+ " "+f.length()));
+        LongAdder la = new LongAdder();
+        foundFiles.forEach(f -> la.add(f.length()));
+        //System.out.println("Sum: "+la.sum());
+
+        assertTrue(la.sum() < sizeCap);
+
+        checkFileCount(expectedFileCount + 1);
     }
 
     @Test
     public void dailyChronologSizeBasedRollover() {
         SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object> sizeAndTimeBasedFileNamingAndTriggeringPolicy = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object>();
         sizeAndTimeBasedFileNamingAndTriggeringPolicy.setMaxFileSize(new FileSize(10000));
-        sizeAndTimeBasedFileNamingAndTriggeringPolicy.invocationGate = fixedRateInvocationGate;
+        //sizeAndTimeBasedFileNamingAndTriggeringPolicy.invocationGate = fixedRateInvocationGate;
         tbfnatp = sizeAndTimeBasedFileNamingAndTriggeringPolicy;
         slashCount = 1;
         String fileNamePattern = randomOutputDir + "/%d{" + DAILY_DATE_PATTERN + "}/clean.%i.zip";
@@ -323,7 +333,7 @@ public void dailyChronologSizeBasedRollover() {
     public void dailyChronologSizeBasedRolloverWithSecondPhase() {
         SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object> sizeAndTimeBasedFileNamingAndTriggeringPolicy = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object>();
         sizeAndTimeBasedFileNamingAndTriggeringPolicy.setMaxFileSize(new FileSize(10000));
-        sizeAndTimeBasedFileNamingAndTriggeringPolicy.invocationGate = fixedRateInvocationGate;
+        //sizeAndTimeBasedFileNamingAndTriggeringPolicy.invocationGate = fixedRateInvocationGate;
         tbfnatp = sizeAndTimeBasedFileNamingAndTriggeringPolicy;
         this.slashCount = 1;
         String fileNamePattern = randomOutputDir + "/%d{" + DAILY_DATE_PATTERN + "}/clean.%i";