moving to counter based file size computation instead of calling file…

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

Hide file tree

Showing 16 changed files with 139 additions and 60 deletions.
diff --git a/...ck-classic/src/test/groovy/ch/qos/logback/classic/gaffer/ConfigurationDelegateTest.groovy b/...ck-classic/src/test/groovy/ch/qos/logback/classic/gaffer/ConfigurationDelegateTest.groovy
@@ -41,7 +41,7 @@ import ch.qos.logback.classic.net.SMTPAppender
 import ch.qos.logback.core.rolling.RollingFileAppender
 import ch.qos.logback.core.rolling.TimeBasedRollingPolicy
 import ch.qos.logback.classic.encoder.PatternLayoutEncoder
-import ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP
+import ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy
 import ch.qos.logback.core.joran.action.TimestampAction
 
 /**
@@ -255,7 +255,7 @@ class ConfigurationDelegateTest {
       file = logFile
       rollingPolicy(TimeBasedRollingPolicy) {
         fileNamePattern = "mylog-%d{yyyy-MM-dd}.%i.txt"
-        timeBasedFileNamingAndTriggeringPolicy(SizeAndTimeBasedFNATP) {
+        timeBasedFileNamingAndTriggeringPolicy(SizeAndTimeBasedFileNamingAndTriggeringPolicy) {
           if(asString)   
             maxFileSize = "100MB"
           else 

diff --git a/logback-classic/src/test/input/fqcn.txt b/logback-classic/src/test/input/fqcn.txt
@@ -753,7 +753,7 @@ ch.qos.logback.core.rolling.RollingFileAppender
 ch.qos.logback.core.rolling.RollingPolicy
 ch.qos.logback.core.rolling.RollingPolicyBase
 ch.qos.logback.core.rolling.RolloverFailure
-ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP
+ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy
 ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy
 ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy
 ch.qos.logback.core.rolling.TimeBasedFileNamingAndTriggeringPolicy

diff --git a/logback-classic/src/test/input/joran/rolling/depratedSizeAndTimeBasedFNATPWarning.xml b/logback-classic/src/test/input/joran/rolling/depratedSizeAndTimeBasedFNATPWarning.xml
@@ -9,7 +9,7 @@
         ${randomOutputDir}${testId}-%d{yyyy-MM-dd_HH_mm_ss}.%i
       </FileNamePattern>
       <TimeBasedFileNamingAndTriggeringPolicy
-        class="ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP">
+        class="ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy">
         <MaxFileSize>100</MaxFileSize>
       </TimeBasedFileNamingAndTriggeringPolicy>
     </rollingPolicy>

diff --git a/logback-classic/src/test/input/joran/rolling/timeAndSizeWithoutMaxFileSize.xml b/logback-classic/src/test/input/joran/rolling/timeAndSizeWithoutMaxFileSize.xml
@@ -10,7 +10,7 @@
 			<fileNamePattern>c:/tmp/logs/rolling-test-%d{yyyy-MM-dd}.%i.log
 			</fileNamePattern>
 			<timeBasedFileNamingAndTriggeringPolicy
-				class="ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP" />
+				class="ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy" />
 			<maxHistory>30</maxHistory>
 			<cleanHistoryOnStart>true</cleanHistoryOnStart>
 		</rollingPolicy>

diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/sift/SiftingAppenderTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/sift/SiftingAppenderTest.java
@@ -29,7 +29,7 @@
 import ch.qos.logback.core.joran.spi.JoranException;
 import ch.qos.logback.core.read.ListAppender;
 import ch.qos.logback.core.rolling.RollingFileAppender;
-import ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP;
+import ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy;
 import ch.qos.logback.core.rolling.TimeBasedRollingPolicy;
 import ch.qos.logback.core.sift.AppenderFactory;
 import ch.qos.logback.core.sift.AppenderTracker;
@@ -42,12 +42,10 @@
 
 import ch.qos.logback.core.testUtil.StringListAppender;
 import ch.qos.logback.core.util.FileSize;
-import ch.qos.logback.core.util.StatusPrinter;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
-import org.slf4j.MDC;
 
 import java.util.List;
 
@@ -358,7 +356,7 @@ public Appender<ILoggingEvent> buildAppender(Context context, String discriminat
                 policy.setParent(appender);
                 policy.setCleanHistoryOnStart(true);
 
-                SizeAndTimeBasedFNATP<ILoggingEvent> innerpolicy = new SizeAndTimeBasedFNATP<ILoggingEvent>();
+                SizeAndTimeBasedFileNamingAndTriggeringPolicy<ILoggingEvent> innerpolicy = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<ILoggingEvent>();
                 innerpolicy.setContext(context);
                 innerpolicy.setMaxFileSize(FileSize.valueOf("5KB"));
                 innerpolicy.setTimeBasedRollingPolicy(policy);

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/LengthCounter.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/LengthCounter.java
@@ -0,0 +1,22 @@
+/*
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * 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
+ * the Eclipse Foundation
+ *
+ *   or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+
+package ch.qos.logback.core.rolling;
+
+public interface LengthCounter {
+
+    void add(long len);
+    long getLength();
+    void reset();
+}
diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/LengthCounterBase.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/LengthCounterBase.java
@@ -0,0 +1,37 @@
+/*
+ * Logback: the reliable, generic, fast and flexible logging framework.
+ * 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
+ * the Eclipse Foundation
+ *
+ *   or (per the licensee's choosing)
+ *
+ * under the terms of the GNU Lesser General Public License version 2.1
+ * as published by the Free Software Foundation.
+ */
+
+package ch.qos.logback.core.rolling;
+
+import java.util.concurrent.atomic.LongAdder;
+
+public class LengthCounterBase implements LengthCounter {
+
+    LongAdder counter = new LongAdder();
+
+    @Override
+    public void add(long len) {
+        counter.add(len);
+    }
+
+    @Override
+    public long getLength() {
+        return counter.longValue();
+    }
+
+    @Override
+    public void reset() {
+        counter.reset();
+    }
+}
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
@@ -266,15 +266,15 @@ public TriggeringPolicy<E> getTriggeringPolicy() {
      */
     @SuppressWarnings("unchecked")
     public void setRollingPolicy(RollingPolicy policy) {
-        if (rollingPolicy instanceof TriggeringPolicy) {
+        if (this.rollingPolicy instanceof TriggeringPolicy) {
             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);
         }
-        rollingPolicy = policy;
-        if (rollingPolicy instanceof TriggeringPolicy) {
-            triggeringPolicy = (TriggeringPolicy<E>) policy;
+        this.rollingPolicy = policy;
+        if (this.rollingPolicy instanceof TriggeringPolicy) {
+            this.triggeringPolicy = (TriggeringPolicy<E>) policy;
         }
 
     }

diff --git a/...k/core/rolling/SizeAndTimeBasedFNATP.java → ...meBasedFileNamingAndTriggeringPolicy.java b/...k/core/rolling/SizeAndTimeBasedFNATP.java → ...meBasedFileNamingAndTriggeringPolicy.java
@@ -15,7 +15,6 @@
 
 import java.io.File;
 import java.time.Instant;
-import java.util.Date;
 
 import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.joran.spi.NoAutoStart;
@@ -25,21 +24,18 @@
 import ch.qos.logback.core.rolling.helper.SizeAndTimeBasedArchiveRemover;
 import ch.qos.logback.core.util.Duration;
 import ch.qos.logback.core.util.FileSize;
-import ch.qos.logback.core.util.DefaultInvocationGate;
-import ch.qos.logback.core.util.InvocationGate;
-import ch.qos.logback.core.util.SimpleInvocationGate;
 
 /**
  * This class implement {@link TimeBasedFileNamingAndTriggeringPolicy}
  * interface extending {@link TimeBasedFileNamingAndTriggeringPolicyBase}. This class is intended to be nested
- * within a {@link SizeAndTimeBasedFNATP} instance.  However, it can also be instantiated directly for testing purposes.
+ * within a {@link SizeAndTimeBasedFileNamingAndTriggeringPolicy} instance.  However, it can also be instantiated directly for testing purposes.
  *
  * @author Ceki G&uuml;lc&uuml;
  *
  * @param <E>
  */
 @NoAutoStart
-public class SizeAndTimeBasedFNATP<E> extends TimeBasedFileNamingAndTriggeringPolicyBase<E> {
+public class SizeAndTimeBasedFileNamingAndTriggeringPolicy<E> extends TimeBasedFileNamingAndTriggeringPolicyBase<E> {
 
     enum Usage {
         EMBEDDED, DIRECT
@@ -55,16 +51,20 @@ enum Usage {
 
     private final Usage usage;
 
-    InvocationGate invocationGate = new SimpleInvocationGate();
+    //InvocationGate invocationGate = new SimpleInvocationGate();
 
-    public SizeAndTimeBasedFNATP() {
+    public SizeAndTimeBasedFileNamingAndTriggeringPolicy() {
         this(Usage.DIRECT);
     }
 
-    public SizeAndTimeBasedFNATP(Usage usage) {
+    public SizeAndTimeBasedFileNamingAndTriggeringPolicy(Usage usage) {
         this.usage = usage;
     }
 
+    public LengthCounter lengthCounter = new LengthCounterBase();
+
+
+
     @Override
     public void start() {
         // we depend on certain fields having been initialized in super class
@@ -83,8 +83,8 @@ public void start() {
             withErrors();
         }
 
-        if (checkIncrement != null)
-            invocationGate = new SimpleInvocationGate(checkIncrement);
+        //if (checkIncrement != null)
+        //    invocationGate = new SimpleInvocationGate(checkIncrement);
 
         if (!validateDateAndIntegerTokens()) {
             withErrors();
@@ -161,18 +161,21 @@ public boolean isTriggeringEvent(File activeFile, final E event) {
                     instantInElapsedPeriod, currentPeriodsCounter);
             currentPeriodsCounter = 0;
             setDateInCurrentPeriod(currentTime);
-
+            lengthCounter.reset();
             return true;
         }
 
-        return checkSizeBasedTrigger(activeFile, currentTime);
+        boolean result = checkSizeBasedTrigger(activeFile, currentTime);
+        if(result)
+            lengthCounter.reset();
+        return result;
     }
 
     private boolean checkSizeBasedTrigger(File activeFile, long currentTime) {
         // next check for roll-over based on size
-        if (invocationGate.isTooSoon(currentTime)) {
-            return false;
-        }
+        //if (invocationGate.isTooSoon(currentTime)) {
+        //    return false;
+        //}
 
         if (activeFile == null) {
             addWarn("activeFile == null");
@@ -182,11 +185,15 @@ private boolean checkSizeBasedTrigger(File activeFile, long currentTime) {
             addWarn("maxFileSize = null");
             return false;
         }
-        if (activeFile.length() >= maxFileSize.getSize()) {
+
+
+
+        if (lengthCounter.getLength() >= maxFileSize.getSize()) {
 
             elapsedPeriodsFileName = tbrp.fileNamePatternWithoutCompSuffix.convertMultipleArguments(dateInCurrentPeriod,
                     currentPeriodsCounter);
             currentPeriodsCounter++;
+
             return true;
         }
 
@@ -211,4 +218,8 @@ public void setMaxFileSize(FileSize aMaxFileSize) {
         this.maxFileSize = aMaxFileSize;
     }
 
+    @Override
+    public LengthCounter getLengthCounter() {
+        return lengthCounter;
+    }
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/SizeAndTimeBasedRollingPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/SizeAndTimeBasedRollingPolicy.java
@@ -13,7 +13,7 @@
  */
 package ch.qos.logback.core.rolling;
 
-import ch.qos.logback.core.rolling.SizeAndTimeBasedFNATP.Usage;
+import ch.qos.logback.core.rolling.SizeAndTimeBasedFileNamingAndTriggeringPolicy.Usage;
 import ch.qos.logback.core.util.FileSize;
 
 public class SizeAndTimeBasedRollingPolicy<E> extends TimeBasedRollingPolicy<E> {
@@ -22,7 +22,7 @@ public class SizeAndTimeBasedRollingPolicy<E> extends TimeBasedRollingPolicy<E>
 
     @Override
     public void start() {
-        SizeAndTimeBasedFNATP<E> sizeAndTimeBasedFNATP = new SizeAndTimeBasedFNATP<E>(Usage.EMBEDDED);
+        SizeAndTimeBasedFileNamingAndTriggeringPolicy<E> sizeAndTimeBasedFNATP = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<E>(Usage.EMBEDDED);
         if (maxFileSize == null) {
             addError("maxFileSize property is mandatory.");
             return;

diff --git a/...src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicyBase.java b/...src/main/java/ch/qos/logback/core/rolling/TimeBasedFileNamingAndTriggeringPolicyBase.java
@@ -17,7 +17,6 @@
 
 import java.io.File;
 import java.time.Instant;
-import java.util.Date;
 import java.util.Locale;
 import java.util.TimeZone;
 import java.util.concurrent.atomic.AtomicLong;
@@ -32,7 +31,7 @@
  * Base implementation of {@link TimeBasedFileNamingAndTriggeringPolicy}.
  *
  * <p>See also derived classes {@link DefaultTimeBasedFileNamingAndTriggeringPolicy} and
- * {@link SizeAndTimeBasedFNATP}.
+ * {@link SizeAndTimeBasedFileNamingAndTriggeringPolicy}.
  *
  * </p>
  *

diff --git a/logback-core/src/main/java/ch/qos/logback/core/rolling/TriggeringPolicy.java b/logback-core/src/main/java/ch/qos/logback/core/rolling/TriggeringPolicy.java
@@ -14,6 +14,8 @@
 package ch.qos.logback.core.rolling;
 
 import java.io.File;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.LongAdder;
 
 import ch.qos.logback.core.spi.LifeCycle;
 
@@ -27,6 +29,17 @@
 
 public interface TriggeringPolicy<E> extends LifeCycle {
 
+    /**
+     *  Return the {@link LengthCounter} instance associated with this triggering
+     *  policy. The returned value may be null.
+     *
+     * @return a LengthCounter instance, may be null
+     * @since 1.5.8
+     */
+    default LengthCounter getLengthCounter() {
+        return null;
+    }
+
     /**
      * Should roll-over be triggered at this time?
      * 

diff --git a/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java b/logback-core/src/test/java/ch/qos/logback/core/rolling/SizeAndTimeBasedFNATP_Test.java
@@ -21,7 +21,6 @@
 import java.util.function.UnaryOperator;
 
 import ch.qos.logback.core.util.Duration;
-import ch.qos.logback.core.util.StatusPrinter;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -35,7 +34,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 
 public class SizeAndTimeBasedFNATP_Test extends ScaffoldingForRollingTests {
-    private SizeAndTimeBasedFNATP<Object> sizeAndTimeBasedFNATP = null;
+    private SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object> sizeAndTimeBasedFNATP = null;
     private RollingFileAppender<Object> rfa1 = new RollingFileAppender<Object>();
     private TimeBasedRollingPolicy<Object> tbrp1 = new TimeBasedRollingPolicy<Object>();
     private RollingFileAppender<Object> rfa2 = new RollingFileAppender<Object>();
@@ -61,7 +60,7 @@ private void initRollingFileAppender(RollingFileAppender<Object> rfa, String fil
 
     private void initPolicies(RollingFileAppender<Object> rfa, TimeBasedRollingPolicy<Object> tbrp,
             String filenamePattern, int sizeThreshold, long givenTime, long lastCheck) {
-        sizeAndTimeBasedFNATP = new SizeAndTimeBasedFNATP<Object>();
+        sizeAndTimeBasedFNATP = new SizeAndTimeBasedFileNamingAndTriggeringPolicy<Object>();
         sizeAndTimeBasedFNATP.setCheckIncrement(Duration.buildByMilliseconds(10));
         tbrp.setContext(context);
         sizeAndTimeBasedFNATP.setMaxFileSize(new FileSize(sizeThreshold));