You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2017/11/10 13:46:25 UTC
logging-log4j2 git commit: LOG4J2-2106 Reduce locking when checking
for rollover
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 <rg...@apache.org>
Authored: Fri Nov 10 06:46:11 2017 -0700
Committer: Ralph Goers <rg...@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>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when
checking for rollover
Posted by Remko Popma <re...@gmail.com>.
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 <da...@shapeblue.com>
wrote:
> Thanks, I'm trying the commitish build install use.
>
> Sent from Nine <http://www.9folders.com/>
> ------------------------------
> *From:* Ralph Goers <ra...@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 <da...@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" <ra...@dslextreme.com> wrote:
> >
> > Ok. I’m waiting on feedback on this before I start a release.
> >
> > Ralph
> >
> >
> >
> > Senior Software Developer
> > daan.hoogland@shapeblue.com
> > www.shapeblue.com
> >
> >
> >
> >> On Nov 11, 2017, at 7:14 AM, Remko Popma <re...@gmail.com> wrote:
> >>
> >> I’ll take a look tomorrow.
> >>
> >>
> >>
> >>> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
> >>>> Authored: Fri Nov 10 06:46:11 2017 -0700
> >>>> Committer: Ralph Goers <rg...@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.hoogland@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/>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking
when checking for rollover
Posted by Daan Hoogland <da...@shapeblue.com>.
Thanks, I'm trying the commitish build install use.
Sent from Nine<http://www.9folders.com/>
________________________________
From: Ralph Goers <ra...@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
Senior Software Developer
daan.hoogland@shapeblue.com
www.shapeblue.com
> On Nov 11, 2017, at 10:59 AM, Daan Hoogland <da...@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" <ra...@dslextreme.com> wrote:
>
> Ok. I’m waiting on feedback on this before I start a release.
>
> Ralph
>
>
>
> Senior Software Developer
> daan.hoogland@shapeblue.com
> www.shapeblue.com<http://www.shapeblue.com>
>
>
>
>> On Nov 11, 2017, at 7:14 AM, Remko Popma <re...@gmail.com> wrote:
>>
>> I’ll take a look tomorrow.
>>
>>
>>
>>> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
>>>> Authored: Fri Nov 10 06:46:11 2017 -0700
>>>> Committer: Ralph Goers <rg...@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>
>>>>
>>>>
>>>
>>>
>>
>
>
>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when
checking for rollover
Posted by Ralph Goers <ra...@dslextreme.com>.
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 <da...@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" <ra...@dslextreme.com> wrote:
>
> Ok. I’m waiting on feedback on this before I start a release.
>
> Ralph
>
>
>
> Senior Software Developer
> daan.hoogland@shapeblue.com
> www.shapeblue.com
>
>
>
>> On Nov 11, 2017, at 7:14 AM, Remko Popma <re...@gmail.com> wrote:
>>
>> I’ll take a look tomorrow.
>>
>>
>>
>>> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
>>>> Authored: Fri Nov 10 06:46:11 2017 -0700
>>>> Committer: Ralph Goers <rg...@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>
>>>>
>>>>
>>>
>>>
>>
>
>
>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking
when checking for rollover
Posted by Daan Hoogland <da...@shapeblue.com>.
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" <ra...@dslextreme.com> wrote:
Ok. I’m waiting on feedback on this before I start a release.
Ralph
Senior Software Developer
daan.hoogland@shapeblue.com
www.shapeblue.com
> On Nov 11, 2017, at 7:14 AM, Remko Popma <re...@gmail.com> wrote:
>
> I’ll take a look tomorrow.
>
>
>
>> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
>>> Authored: Fri Nov 10 06:46:11 2017 -0700
>>> Committer: Ralph Goers <rg...@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>
>>>
>>>
>>
>>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when
checking for rollover
Posted by Ralph Goers <ra...@dslextreme.com>.
Ok. I’m waiting on feedback on this before I start a release.
Ralph
> On Nov 11, 2017, at 7:14 AM, Remko Popma <re...@gmail.com> wrote:
>
> I’ll take a look tomorrow.
>
>
>
>> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
>>> Authored: Fri Nov 10 06:46:11 2017 -0700
>>> Committer: Ralph Goers <rg...@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>
>>>
>>>
>>
>>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when checking for rollover
Posted by Remko Popma <re...@gmail.com>.
I’ll take a look tomorrow.
> On Nov 10, 2017, at 22:47, Ralph Goers <rg...@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, rgoers@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 <rg...@apache.org>
>> Authored: Fri Nov 10 06:46:11 2017 -0700
>> Committer: Ralph Goers <rg...@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>
>>
>>
>
>
Re: logging-log4j2 git commit: LOG4J2-2106 Reduce locking when
checking for rollover
Posted by Ralph Goers <rg...@apache.org>.
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, rgoers@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 <rg...@apache.org>
> Authored: Fri Nov 10 06:46:11 2017 -0700
> Committer: Ralph Goers <rg...@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>
>
>