I reviewed the changes and found some potential issues. Please see my comment on https://issues.apache.org/jira/browse/LOG4J2-2106
On Sun, Nov 12, 2017 at 6:17 AM, Daan Hoogland <daan.hoogl...@shapeblue.com> wrote: > Thanks, I'm trying the commitish build install use. > > Sent from Nine <http://www.9folders.com/> > ------------------------------ > *From:* Ralph Goers <ralph.go...@dslextreme.com> > *Sent:* 11 Nov 2017 8:45 pm > *To:* dev@logging.apache.org > *Subject:* Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when > checking for rollover > > Release candidates are only available on the Apache staging repository. > The download information will be in the email for the release candidate. > > Ralph > > > > On Nov 11, 2017, at 10:59 AM, Daan Hoogland <daan.hoogl...@shapeblue.com> > wrote: > > > > As a matter of interest, can we test release candidates from maven > central at apache or do I need to build and install locally to test? > > > > thanks > > > > On 11/11/2017, 18:32, "Ralph Goers" <ralph.go...@dslextreme.com> wrote: > > > > Ok. I’m waiting on feedback on this before I start a release. > > > > Ralph > > > > > > > > Senior Software Developer > > daan.hoogl...@shapeblue.com > > www.shapeblue.com > > > > > > > >> 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> > >>>> > >>>> > >>> > >>> > >> > > > > > > > > > > > > > Daan Hoogland > Senior Software Developer > *s:* +31 61400 4545 | * d: *+44(0) 20 3603 0540 <+44%2020%203603%200540> > *e:* daan.hoogl...@shapeblue.com | * w: *www.shapeblue.com |* t:* > @shapeblue > *a:* 53 Chandos Place, Covent Garden, London WC2N > <https://maps.google.com/?q=53+Chandos+Place,+Covent+Garden,+London+WC2N&entry=gmail&source=g> > 4HSUK > ------------------------------ > > Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue > Services India LLP is a company incorporated in India and is operated under > license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a > company incorporated in Brasil and is operated under license from Shape > Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of > South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is > a registered trademark.This email and any attachments to it may be > confidential and are intended solely for the use of the individual to whom > it is addressed. Any views or opinions expressed are solely those of the > author and do not necessarily represent those of Shape Blue Ltd or related > companies. If you are not the intended recipient of this email, you must > neither take any action based upon its contents, nor copy or show it to > anyone. Please contact the sender if you believe you have received this > email in error. > > Find out more about ShapeBlue and our range of CloudStack related services: > IaaS Cloud Design & Build > <http://shapeblue.com/iaas-cloud-design-and-build/> | CSForge – rapid > IaaS deployment framework <http://shapeblue.com/csforge/> > CloudStack Consulting <http://shapeblue.com/cloudstack-consultancy/> | > CloudStack > Software Engineering > <http://shapeblue.com/cloudstack-software-engineering/> > CloudStack Infrastructure Support > <http://shapeblue.com/cloudstack-infrastructure-support/> | CloudStack > Bootcamp Training Courses <http://shapeblue.com/cloudstack-training/> >
