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> >> >> > >
