[
https://issues.apache.org/jira/browse/LOG4J2-2106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16248918#comment-16248918
]
Ralph Goers edited comment on LOG4J2-2106 at 11/12/17 5:16 PM:
---------------------------------------------------------------
That is a good question. Both rollover and checkRollover synchronize on the
FileManager. So all calls to checkRollover are synchronized and will be blocked
by rollover. rollover calls rollover(strategy). It waits for any async tasks to
be completed before it will execute (to prevent trying to compress the same
file). This will cause a second call to rollover to block until all async
threads are complete, which will cause threads to block waiting to log.
The logic in rollover(strategy) of waiting for the async thread to complete is
correct and is required. But I don't believe logging should be blocked while
the rollover completes. So this may only partially solve the reporter's issue.
*Potential Issue 1*
I made isTriggeringEvent in both TimeBasedTriggeringPolicy and
SizeBasedTriggeringPolicy synchronized. However, both update the file time so
both need to use the same lock. I will need to create a lock for them to
synchronize on.
*Potential Issue 2*
That is really the same as Potential Issue 1 and the same fix should apply.
*Optimization*
I don't think that will work as the call to getNextTime needs to be locked
against changes by calls to updateTime from SizeBasedTriggeringPolicy. That
could be done by synchronizing those methods.
But I now see another issue. It seems that we allow the patternProcessor to be
replaced on a reconfiguration. Currently it copies the time values from the old
pattern processor to the new one. But that only works properly if the pattern
processor time values cannot be changed after the new copy is made or only use
the new PatternProcessor after the values are copied. Right now updateData
isn't synchronized at all. Yes, the TriggeringPolicyUpdater shouldn't be
necessary.
*Idea for future enhancement*
I am not sure why this needs to be a future enhancement. setTriggeringPolicy
should start the new Policy before it is set in the FileManager. There still
could be threads trying to use the TriggeringPolicy after stop is called, but
for the current TriggeringPolicies that won't cause a problem.
was (Author: ralph.go...@dslextreme.com):
That is a good question. Both rollover and checkRollover synchronize on the
FileManager. So all calls to checkRollover are synchronized and will be blocked
by rollover. rollover calls rollover(strategy). It waits for any async tasks to
be completed before it will execute (to prevent trying to compress the same
file). This will cause a second call to rollover to block until all async
threads are complete, which will cause threads to block waiting to log.
The logic in rollover(strategy) of waiting for the async thread to complete is
correct and is required. But I don't believe logging should be blocked while
the rollover completes. So this may only partially solve the reporter's issue.
*Potential Issue 1*
I made isTriggeringEvent in both TimeBasedTriggeringPolicy and
SizeBasedTriggeringPolicy synchronized. However, both update the file time so
both need to use the same lock. I will need to create a lock for them to
synchronize on.
*Potential Issue 2*
That is really the same as Potential Issue 1 and the same fix should apply.
*Optimization*
I don't think that will work as the call to getNextTime needs to be locked
against changes by calls to updateTime from SizeBasedTriggeringPolicy. That
could be done by synchronizing those methods.
But I now see another issue. It seems that we allow the patternProcessor to be
replaced on a reconfiguration. Currently it copies the time values from the old
pattern processor to the new one. But that only works properly if the pattern
processor time values cannot be changed after the new copy is made or only use
the new PatternProcessor after the values are copied. Right now updateData
isn't synchronized at all.
*Idea for future enhancement*
I am not sure why this needs to be a future enhancement. setTriggeringPolicy
should start the new Policy before it is set in the FileManager. There still
could be threads trying to use the TriggeringPolicy after stop is called, but
for the current TriggeringPolicies that won't cause a problem.
> May blocked when two compress action
> ------------------------------------
>
> Key: LOG4J2-2106
> URL: https://issues.apache.org/jira/browse/LOG4J2-2106
> Project: Log4j 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 2.9.1
> Reporter: skopt
>
> My log file is very large, so I set the config about compression. The
> policies is as follows:
> <Policies>
> <OnStartupTriggeringPolicy/>
> <TimeBasedTriggeringPolicy/>
> </Policies>
> When restart the service on the hour, this will trigger the two
> policies. And I find that the log can not be written for a while.
> I think, there would be two compress actions. In the file
> RollingFileManager(package org.apache.logging.log4j.core.appender.rolling),
> the actions will submit to asyncExecutor which will create a new thread for
> every work. That mean the log file will be compressed by the two actions. Is
> this the cause?
> Thanks.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
