Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java

2014-04-28 Thread Matt Sicker
I'll keep it in mind not to do pointless lock changes. Doing a synchronized(someObject) is perfectly fine, too. On 28 April 2014 22:21, Ralph Goers wrote: > I think you are still missing the point. Whether I agree with the > arguments or not, I still don't think it is worth the time to change

Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java

2014-04-28 Thread Ralph Goers
I think you are still missing the point. Whether I agree with the arguments or not, I still don't think it is worth the time to change stuff like this that is already working just because it is considered a better style. You have a better argument with the synchronization stuff because it coul

Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java

2014-04-28 Thread Matt Sicker
Here, I found a debate over what I was trying to convey: http://stackoverflow.com/questions/541487/implements-runnable-vs-extends-thread On 28 April 2014 12:22, Ralph Goers wrote: > Preferring Executors to Threads really has nothing to do with the change > below since there isn’t an Executor i

Re: Question about some code in ClassLoaderContextSelector.

2014-04-28 Thread Ralph Goers
I suspect it is because a) the LoggerContext will need to be configured and probably doesn't have a configuration so it will just use the default and b) the default LoggerContext would probably never be used. Ralph > On Apr 28, 2014, at 4:36 PM, Matt Sicker wrote: > > For convenience, here is

Question about some code in ClassLoaderContextSelector.

2014-04-28 Thread Matt Sicker
For convenience, here is the code snippets I'm looking at. private static final AtomicReference CONTEXT = new AtomicReference(); // ... private LoggerContext getDefault() { final LoggerContext ctx = CONTEXT.get(); if (ctx != null) { return ctx; }

Proposal to move some interfaces in log4j-core.

2014-04-28 Thread Matt Sicker
Pretty much all the interfaces in log4j-core except for: * jmx * web * maybe some subpackages in appender (e.g., rewrite, rolling) I'm proposing we put all these public API classes into either o.a.l.l.core or making a new package for it such as o.a.l.l.core.api. This can aid in making it obvious w

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
I'll delegate my arguments to the SO post about it: http://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java In short, it's defensive programming. It's safer to synchronize on an internal object than on this. Plus, it aids in concurrency throughput instead of just using a synchroni

Splitting off NoSQL to its own module.

2014-04-28 Thread Matt Sicker
After some basic discussion about this, I'm going to split off the NoSQL code into a module called "log4j-nosql". This is where the Mongo and Couch plugins are right now, and it looks like we'll be adding a Solr one soon most likely along with that Gora one. Anyway, I split it out last night but d

Re: Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
Why are they not appropriate lock objects? Start a discussion before just changing them. Ralph On Apr 28, 2014, at 10:40 AM, Matt Sicker wrote: > In that case, Item 69: prefer concurrency utilities to wait and notify. > Sounds like we can just use a plain Object instance to lock (which is p

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
In that case, Item 69: prefer concurrency utilities to wait and notify. Sounds like we can just use a plain Object instance to lock (which is pretty much equivalent to using ReentrantLock) when doing normal locks, but instead of using .notifyAll() and .wait(), we should use the Condition interface

[jira] [Assigned] (LOG4J2-620) Deadlock on reconfiguration with Appenders that use log4j

2014-04-28 Thread Ralph Goers (JIRA)
[ https://issues.apache.org/jira/browse/LOG4J2-620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ralph Goers reassigned LOG4J2-620: -- Assignee: Ralph Goers > Deadlock on reconfiguration with Appenders that use log4j > ---

Re: Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
Yes, guidelines called out by Effective Java are appropriate when they apply. As for concurrency, “New” isn’t always better than old. In a few places you changed synchronized(object) to use a Lock instead. There is little to no value in doing that and makes the code look a little more cluttered

Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java

2014-04-28 Thread Ralph Goers
Preferring Executors to Threads really has nothing to do with the change below since there isn’t an Executor in sight in the old or the new. If you want a pool of Threads then item 68 applies. For a single thread creating a Thread with a run method is OK. So is the way you did it - it is just m

[jira] [Updated] (LOG4J2-620) Deadlock on reconfiguration with Appenders that use log4j

2014-04-28 Thread Stefan Wehner (JIRA)
[ https://issues.apache.org/jira/browse/LOG4J2-620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Stefan Wehner updated LOG4J2-620: - Description: We're using the JDBC appender to log to database and provide connections from a C3P

[jira] [Updated] (LOG4J2-620) Deadlock on reconfiguration with Appenders that use log4j

2014-04-28 Thread Stefan Wehner (JIRA)
[ https://issues.apache.org/jira/browse/LOG4J2-620?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Stefan Wehner updated LOG4J2-620: - Attachment: real_app_trace.txt sample_app_trace.txt deadlock.tgz

[jira] [Created] (LOG4J2-620) Deadlock on reconfiguration with Appenders that use log4j

2014-04-28 Thread Stefan Wehner (JIRA)
Stefan Wehner created LOG4J2-620: Summary: Deadlock on reconfiguration with Appenders that use log4j Key: LOG4J2-620 URL: https://issues.apache.org/jira/browse/LOG4J2-620 Project: Log4j 2 Iss

Re: Log4j Project Guidelines

2014-04-28 Thread Matt Sicker
What about style things covered by Effective Java? These are pretty much all good ideas. Also, how about some guidelines on concurrency? I'd recommend not using the old concurrency stuff and instead using java.util.concurrent.* for more effective concurrency. This doesn't include the Thread vs Run

Re: svn commit: r1590447 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/CachedClock.java

2014-04-28 Thread Matt Sicker
It's based on Item 68 from Effective Java (prefer executors and tasks to threads), and those use Runnable and Callable instead of Thread. Plus it's simpler to implement Runnable and construct a Thread from it if necessary. Overall, it's better to use the Executors class to spawn off threads. On 2

Re: Log4j Project Guidelines

2014-04-28 Thread Gary Gregory
Perhaps if one breaks the build, it should be polite to revert that last commit... Gary On Mon, Apr 28, 2014 at 3:07 AM, Ralph Goers wrote: > I think we need to develop and post some development “guidelines”, “best > practices” or whatever you want to call it for Log4j 2. Here are some of > th

Re: Log4j Project Guidelines

2014-04-28 Thread Paul Benedict
I recommend this gets reposted to the wiki or the site docs. On Mon, Apr 28, 2014 at 2:07 AM, Ralph Goers wrote: > I think we need to develop and post some development “guidelines”, “best > practices” or whatever you want to call it for Log4j 2. Here are some of > the things I would definitely

[jira] [Created] (LOG4J2-619) Unable to recover after loading corrupted XML

2014-04-28 Thread Sergey Burkov (JIRA)
Sergey Burkov created LOG4J2-619: Summary: Unable to recover after loading corrupted XML Key: LOG4J2-619 URL: https://issues.apache.org/jira/browse/LOG4J2-619 Project: Log4j 2 Issue Type: Bug

Log4j Project Guidelines

2014-04-28 Thread Ralph Goers
I think we need to develop and post some development “guidelines”, “best practices” or whatever you want to call it for Log4j 2. Here are some of the things I would definitely want on it. 1. Error on the side of caution. If you don’t understand it, don’t touch it and ask on the list. If you t