Ok. I’m waiting on feedback on this before I start a release. Ralph
> On Nov 11, 2017, at 7:14 AM, Remko Popma <remko.po...@gmail.com> wrote: > > I’ll take a look tomorrow. > > > >> On Nov 10, 2017, at 22:47, Ralph Goers <rgo...@apache.org> wrote: >> >> Please review this commit as I want to make sure I didn’t make any mistakes. >> >> Ralph >> >> >>> On Nov 10, 2017, at 6:46 AM, rgo...@apache.org wrote: >>> >>> Repository: logging-log4j2 >>> Updated Branches: >>> refs/heads/master 0dca987fc -> aad2f132b >>> >>> >>> LOG4J2-2106 Reduce locking when checking for rollover >>> >>> >>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>> Commit: >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/aad2f132 >>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/aad2f132 >>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/aad2f132 >>> >>> Branch: refs/heads/master >>> Commit: aad2f132b27f9e2667c2b43fb58ce59e3914edb3 >>> Parents: 0dca987 >>> Author: Ralph Goers <rgo...@apache.org> >>> Authored: Fri Nov 10 06:46:11 2017 -0700 >>> Committer: Ralph Goers <rgo...@apache.org> >>> Committed: Fri Nov 10 06:46:11 2017 -0700 >>> >>> ---------------------------------------------------------------------- >>> .../appender/rolling/RollingFileManager.java | 57 +++++++++++++------- >>> .../rolling/SizeBasedTriggeringPolicy.java | 2 +- >>> .../rolling/TimeBasedTriggeringPolicy.java | 2 +- >>> src/changes/changes.xml | 3 ++ >>> 4 files changed, 42 insertions(+), 22 deletions(-) >>> ---------------------------------------------------------------------- >>> >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/aad2f132/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java >>> ---------------------------------------------------------------------- >>> diff --git >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java >>> >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java >>> index 6ccfe7b..ff7bf6d 100644 >>> --- >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java >>> +++ >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/RollingFileManager.java >>> @@ -29,6 +29,9 @@ import java.util.concurrent.Semaphore; >>> import java.util.concurrent.ThreadPoolExecutor; >>> import java.util.concurrent.TimeUnit; >>> import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; >>> +import java.util.concurrent.locks.Lock; >>> +import java.util.concurrent.locks.ReadWriteLock; >>> +import java.util.concurrent.locks.ReentrantReadWriteLock; >>> >>> import org.apache.logging.log4j.core.Layout; >>> import org.apache.logging.log4j.core.LifeCycle; >>> @@ -65,6 +68,9 @@ public class RollingFileManager extends FileManager { >>> private volatile boolean initialized = false; >>> private volatile String fileName; >>> private final FileExtension fileExtension; >>> + private final ReadWriteLock updateLock = new ReentrantReadWriteLock(); >>> + private final Lock readLock = updateLock.readLock(); >>> + private final Lock writeLock = updateLock.writeLock(); >>> >>> /* This executor pool will create a new Thread for every work async >>> action to be performed. Using it allows >>> us to make sure all the Threads are completed when the Manager is >>> stopped. */ >>> @@ -247,9 +253,14 @@ public class RollingFileManager extends FileManager { >>> * Determines if a rollover should occur. >>> * @param event The LogEvent. >>> */ >>> - public synchronized void checkRollover(final LogEvent event) { >>> - if (triggeringPolicy.isTriggeringEvent(event)) { >>> - rollover(); >>> + public void checkRollover(final LogEvent event) { >>> + readLock.lock(); >>> + try { >>> + if (triggeringPolicy.isTriggeringEvent(event)) { >>> + rollover(); >>> + } >>> + } finally { >>> + readLock.unlock(); >>> } >>> } >>> >>> @@ -333,25 +344,31 @@ public class RollingFileManager extends FileManager { >>> } >>> >>> public void setTriggeringPolicy(final TriggeringPolicy triggeringPolicy) { >>> - triggeringPolicy.initialize(this); >>> - final TriggeringPolicy policy = this.triggeringPolicy; >>> - int count = 0; >>> - boolean policyUpdated = false; >>> - do { >>> - ++count; >>> - } while (!(policyUpdated = >>> triggeringPolicyUpdater.compareAndSet(this, this.triggeringPolicy, >>> triggeringPolicy)) >>> - && count < MAX_TRIES); >>> - if (policyUpdated) { >>> - if (triggeringPolicy instanceof LifeCycle) { >>> - ((LifeCycle) triggeringPolicy).start(); >>> - } >>> - if (policy instanceof LifeCycle) { >>> - ((LifeCycle) policy).stop(); >>> + writeLock.lock(); >>> + try { >>> + triggeringPolicy.initialize(this); >>> + final TriggeringPolicy policy = this.triggeringPolicy; >>> + int count = 0; >>> + boolean policyUpdated = false; >>> + do { >>> + ++count; >>> } >>> - } else { >>> - if (triggeringPolicy instanceof LifeCycle) { >>> - ((LifeCycle) triggeringPolicy).stop(); >>> + while (!(policyUpdated = >>> triggeringPolicyUpdater.compareAndSet(this, this.triggeringPolicy, >>> triggeringPolicy)) >>> + && count < MAX_TRIES); >>> + if (policyUpdated) { >>> + if (triggeringPolicy instanceof LifeCycle) { >>> + ((LifeCycle) triggeringPolicy).start(); >>> + } >>> + if (policy instanceof LifeCycle) { >>> + ((LifeCycle) policy).stop(); >>> + } >>> + } else { >>> + if (triggeringPolicy instanceof LifeCycle) { >>> + ((LifeCycle) triggeringPolicy).stop(); >>> + } >>> } >>> + } finally { >>> + writeLock.unlock(); >>> } >>> } >>> >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/aad2f132/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/SizeBasedTriggeringPolicy.java >>> ---------------------------------------------------------------------- >>> diff --git >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/SizeBasedTriggeringPolicy.java >>> >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/SizeBasedTriggeringPolicy.java >>> index f77d571..81069fa 100644 >>> --- >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/SizeBasedTriggeringPolicy.java >>> +++ >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/SizeBasedTriggeringPolicy.java >>> @@ -73,7 +73,7 @@ public class SizeBasedTriggeringPolicy extends >>> AbstractTriggeringPolicy { >>> * @return true if a rollover should take place, false otherwise. >>> */ >>> @Override >>> - public boolean isTriggeringEvent(final LogEvent event) { >>> + public synchronized boolean isTriggeringEvent(final LogEvent event) { >>> final boolean triggered = manager.getFileSize() > maxFileSize; >>> if (triggered) { >>> manager.getPatternProcessor().updateTime(); >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/aad2f132/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java >>> ---------------------------------------------------------------------- >>> diff --git >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java >>> >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java >>> index 7f6ac79..953e186 100644 >>> --- >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java >>> +++ >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java >>> @@ -122,7 +122,7 @@ public final class TimeBasedTriggeringPolicy extends >>> AbstractTriggeringPolicy { >>> * @return true if a rollover should occur. >>> */ >>> @Override >>> - public boolean isTriggeringEvent(final LogEvent event) { >>> + public synchronized boolean isTriggeringEvent(final LogEvent event) { >>> if (manager.getFileSize() == 0) { >>> return false; >>> } >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/aad2f132/src/changes/changes.xml >>> ---------------------------------------------------------------------- >>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml >>> index d54484a..2bcefb2 100644 >>> --- a/src/changes/changes.xml >>> +++ b/src/changes/changes.xml >>> @@ -31,6 +31,9 @@ >>> - "remove" - Removed >>> --> >>> <release version="2.10.0" date="2017-MM-DD" description="GA Release >>> 2.10.0"> >>> + <action issue="LOG4J2-2106" dev="rogers" type="fix"> >>> + Reduce locking when checking for rollover. >>> + </action> >>> <action issue="LOG4J2-2103" dev="mikes" type="add"> >>> XML encoding for PatternLayout. >>> </action> >>> >>> >> >> >
