You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ni...@apache.org on 2014/02/08 00:55:45 UTC
svn commit: r1565858 - in /logging/log4j/log4j2/trunk/log4j-core/src:
main/java/org/apache/logging/log4j/core/appender/db/
main/java/org/apache/logging/log4j/core/appender/db/jdbc/
main/java/org/apache/logging/log4j/core/appender/db/jpa/ main/java/org/...
Author: nickwilliams
Date: Fri Feb 7 23:55:45 2014
New Revision: 1565858
URL: http://svn.apache.org/r1565858
Log:
Refactoring the abstract database manager because connecting once and staying connected for the entire life of the manager is not going to work. Part 1 of 2-part commit for fixing LOG4J2-407, LOG4J2-438, LOG4J2-442, LOG4J2-457, and LOG4J2-489.
Modified:
logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java
logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java
logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JDBCDatabaseManager.java
logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/JPADatabaseManager.java
logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManager.java
logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppenderTest.java
logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java
logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManagerTest.java
Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppender.java Fri Feb 7 23:55:45 2014
@@ -84,7 +84,7 @@ public abstract class AbstractDatabaseAp
}
super.start();
if (this.getManager() != null) {
- this.getManager().connect();
+ this.getManager().startup();
}
}
@@ -125,8 +125,8 @@ public abstract class AbstractDatabaseAp
this.writeLock.lock();
try {
final T old = this.getManager();
- if (!manager.isConnected()) {
- manager.connect();
+ if (!manager.isRunning()) {
+ manager.startup();
}
this.manager = manager;
old.release();
Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java Fri Feb 7 23:55:45 2014
@@ -29,7 +29,7 @@ public abstract class AbstractDatabaseMa
private final ArrayList<LogEvent> buffer;
private final int bufferSize;
- private boolean connected = false;
+ private boolean running = false;
/**
* Instantiates the base manager.
@@ -45,22 +45,25 @@ public abstract class AbstractDatabaseMa
}
/**
- * Implementations should implement this method to perform any proprietary connection operations. This method will
- * never be called twice on the same instance. It is safe to throw any exceptions from this method.
+ * Implementations should implement this method to perform any proprietary startup operations. This method will
+ * never be called twice on the same instance. It is safe to throw any exceptions from this method. This method
+ * does not necessarily connect to the database, as it is generally unreliable to connect once and use the same
+ * connection for hours.
*/
- protected abstract void connectInternal() throws Exception;
+ protected abstract void startupInternal() throws Exception;
/**
* This method is called within the appender when the appender is started. If it has not already been called, it
- * calls {@link #connectInternal()} and catches any exceptions it might throw.
+ * calls {@link #startupInternal()} and catches any exceptions it might throw.
*/
- public final synchronized void connect() {
- if (!this.isConnected()) {
+ public final synchronized void startup() {
+ if (!this.isRunning()) {
try {
- this.connectInternal();
- this.connected = true;
+ this.startupInternal();
+ this.running = true;
} catch (final Exception e) {
- LOGGER.error("Could not connect to database using logging manager [{}].", this.getName(), e);
+ LOGGER.error("Could not perform database startup operations using logging manager [{}].",
+ this.getName(), e);
}
}
}
@@ -68,36 +71,38 @@ public abstract class AbstractDatabaseMa
/**
* Implementations should implement this method to perform any proprietary disconnection / shutdown operations. This
* method will never be called twice on the same instance, and it will only be called <em>after</em>
- * {@link #connectInternal()}. It is safe to throw any exceptions from this method.
+ * {@link #startupInternal()}. It is safe to throw any exceptions from this method. This method does not
+ * necessarily disconnect from the database for the same reasons outlined in {@link #startupInternal()}.
*/
- protected abstract void disconnectInternal() throws Exception;
+ protected abstract void shutdownInternal() throws Exception;
/**
* This method is called from the {@link #release()} method when the appender is stopped or the appender's manager
- * is replaced. If it has not already been called, it calls {@link #disconnectInternal()} and catches any exceptions
+ * is replaced. If it has not already been called, it calls {@link #shutdownInternal()} and catches any exceptions
* it might throw.
*/
- public final synchronized void disconnect() {
+ public final synchronized void shutdown() {
this.flush();
- if (this.isConnected()) {
+ if (this.isRunning()) {
try {
- this.disconnectInternal();
+ this.shutdownInternal();
} catch (final Exception e) {
- LOGGER.warn("Error while disconnecting from database using logging manager [{}].", this.getName(), e);
+ LOGGER.warn("Error while performing database shutdown operations using logging manager [{}].",
+ this.getName(), e);
} finally {
- this.connected = false;
+ this.running = false;
}
}
}
/**
- * Indicates whether the manager is currently connected {@link #connect()} has been called and {@link #disconnect()}
+ * Indicates whether the manager is currently connected {@link #startup()} has been called and {@link #shutdown()}
* has not been called).
*
* @return {@code true} if the manager is connected.
*/
- public final boolean isConnected() {
- return this.connected;
+ public final boolean isRunning() {
+ return this.running;
}
/**
@@ -110,10 +115,10 @@ public abstract class AbstractDatabaseMa
/**
* This method is called automatically when the buffer size reaches its maximum or at the beginning of a call to
- * {@link #disconnect()}. It can also be called manually to flush events to the database.
+ * {@link #shutdown()}. It can also be called manually to flush events to the database.
*/
public final synchronized void flush() {
- if (this.isConnected() && this.buffer.size() > 0) {
+ if (this.isRunning() && this.buffer.size() > 0) {
for (final LogEvent event : this.buffer) {
this.writeInternal(event);
}
@@ -139,7 +144,7 @@ public abstract class AbstractDatabaseMa
@Override
public final void releaseSub() {
- this.disconnect();
+ this.shutdown();
}
@Override
Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JDBCDatabaseManager.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JDBCDatabaseManager.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JDBCDatabaseManager.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JDBCDatabaseManager.java Fri Feb 7 23:55:45 2014
@@ -53,13 +53,13 @@ public final class JDBCDatabaseManager e
}
@Override
- protected void connectInternal() throws SQLException {
+ protected void startupInternal() throws SQLException {
this.connection = this.connectionSource.getConnection();
this.statement = this.connection.prepareStatement(this.sqlStatement);
}
@Override
- protected void disconnectInternal() throws SQLException {
+ protected void shutdownInternal() throws SQLException {
try {
Closer.close(this.statement);
} finally {
@@ -71,7 +71,7 @@ public final class JDBCDatabaseManager e
protected void writeInternal(final LogEvent event) {
StringReader reader = null;
try {
- if (!this.isConnected() || this.connection == null || this.connection.isClosed()) {
+ if (!this.isRunning() || this.connection == null || this.connection.isClosed()) {
throw new AppenderLoggingException(
"Cannot write logging event; JDBC manager not connected to the database.");
}
Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/JPADatabaseManager.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/JPADatabaseManager.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/JPADatabaseManager.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jpa/JPADatabaseManager.java Fri Feb 7 23:55:45 2014
@@ -50,12 +50,12 @@ public final class JPADatabaseManager ex
}
@Override
- protected void connectInternal() {
+ protected void startupInternal() {
this.entityManagerFactory = Persistence.createEntityManagerFactory(this.persistenceUnitName);
}
@Override
- protected void disconnectInternal() {
+ protected void shutdownInternal() {
if (this.entityManagerFactory != null && this.entityManagerFactory.isOpen()) {
this.entityManagerFactory.close();
}
@@ -63,7 +63,7 @@ public final class JPADatabaseManager ex
@Override
protected void writeInternal(final LogEvent event) {
- if (!this.isConnected() || this.entityManagerFactory == null) {
+ if (!this.isRunning() || this.entityManagerFactory == null) {
throw new AppenderLoggingException(
"Cannot write logging event; JPA manager not connected to the database.");
}
Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManager.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManager.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManager.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManager.java Fri Feb 7 23:55:45 2014
@@ -44,12 +44,12 @@ public final class NoSQLDatabaseManager<
}
@Override
- protected void connectInternal() {
+ protected void startupInternal() {
this.connection = this.provider.getConnection();
}
@Override
- protected void disconnectInternal() {
+ protected void shutdownInternal() {
if (this.connection != null && !this.connection.isClosed()) {
this.connection.close();
}
@@ -57,7 +57,7 @@ public final class NoSQLDatabaseManager<
@Override
protected void writeInternal(final LogEvent event) {
- if (!this.isConnected() || this.connection == null || this.connection.isClosed()) {
+ if (!this.isRunning() || this.connection == null || this.connection.isClosed()) {
throw new AppenderLoggingException(
"Cannot write logging event; NoSQL manager not connected to the database.");
}
Modified: logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppenderTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppenderTest.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppenderTest.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseAppenderTest.java Fri Feb 7 23:55:45 2014
@@ -70,7 +70,7 @@ public class AbstractDatabaseAppenderTes
public void testStartAndStop() throws Exception {
this.setUp("name");
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager, this.appender);
@@ -102,7 +102,7 @@ public class AbstractDatabaseAppenderTes
final LocalAbstractDatabaseManager newManager = createMockBuilder(LocalAbstractDatabaseManager.class)
.withConstructor(String.class, int.class).withArgs("name", 0).addMockedMethod("release")
.createStrictMock();
- newManager.connectInternal();
+ newManager.startupInternal();
expectLastCall();
replay(this.manager, this.appender, newManager);
Modified: logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManagerTest.java Fri Feb 7 23:55:45 2014
@@ -40,59 +40,59 @@ public class AbstractDatabaseManagerTest
}
@Test
- public void testConnection01() throws Exception {
+ public void testStartupShutdown01() throws Exception {
this.setUp("testName01", 0);
replay(this.manager);
assertEquals("The name is not correct.", "testName01", this.manager.getName());
- assertFalse("The manager should not be connected.", this.manager.isConnected());
+ assertFalse("The manager should not have started.", this.manager.isRunning());
verify(this.manager);
reset(this.manager);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager);
- this.manager.connect();
- assertTrue("The manager should be connected now.", this.manager.isConnected());
+ this.manager.startup();
+ assertTrue("The manager should be running now.", this.manager.isRunning());
verify(this.manager);
reset(this.manager);
- this.manager.disconnectInternal();
+ this.manager.shutdownInternal();
expectLastCall();
replay(this.manager);
- this.manager.disconnect();
- assertFalse("The manager should not be connected anymore.", this.manager.isConnected());
+ this.manager.shutdown();
+ assertFalse("The manager should not be running anymore.", this.manager.isRunning());
}
@Test
- public void testConnection02() throws Exception {
+ public void testStartupShutdown02() throws Exception {
this.setUp("anotherName02", 0);
replay(this.manager);
assertEquals("The name is not correct.", "anotherName02", this.manager.getName());
- assertFalse("The manager should not be connected.", this.manager.isConnected());
+ assertFalse("The manager should not have started.", this.manager.isRunning());
verify(this.manager);
reset(this.manager);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager);
- this.manager.connect();
- assertTrue("The manager should be connected now.", this.manager.isConnected());
+ this.manager.startup();
+ assertTrue("The manager should be running now.", this.manager.isRunning());
verify(this.manager);
reset(this.manager);
- this.manager.disconnectInternal();
+ this.manager.shutdownInternal();
expectLastCall();
replay(this.manager);
this.manager.releaseSub();
- assertFalse("The manager should not be connected anymore.", this.manager.isConnected());
+ assertFalse("The manager should not be running anymore.", this.manager.isRunning());
}
@Test
@@ -121,13 +121,13 @@ public class AbstractDatabaseManagerTest
final LogEvent event2 = createStrictMock(LogEvent.class);
final LogEvent event3 = createStrictMock(LogEvent.class);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
this.manager.writeInternal(same(event1));
expectLastCall();
replay(this.manager);
- this.manager.connect();
+ this.manager.startup();
this.manager.write(event1);
@@ -157,11 +157,11 @@ public class AbstractDatabaseManagerTest
final LogEvent event3 = createStrictMock(LogEvent.class);
final LogEvent event4 = createStrictMock(LogEvent.class);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager);
- this.manager.connect();
+ this.manager.startup();
this.manager.write(event1);
this.manager.write(event2);
@@ -190,11 +190,11 @@ public class AbstractDatabaseManagerTest
final LogEvent event2 = createStrictMock(LogEvent.class);
final LogEvent event3 = createStrictMock(LogEvent.class);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager);
- this.manager.connect();
+ this.manager.startup();
this.manager.write(event1);
this.manager.write(event2);
@@ -221,11 +221,11 @@ public class AbstractDatabaseManagerTest
final LogEvent event2 = createStrictMock(LogEvent.class);
final LogEvent event3 = createStrictMock(LogEvent.class);
- this.manager.connectInternal();
+ this.manager.startupInternal();
expectLastCall();
replay(this.manager);
- this.manager.connect();
+ this.manager.startup();
this.manager.write(event1);
this.manager.write(event2);
@@ -239,10 +239,10 @@ public class AbstractDatabaseManagerTest
expectLastCall();
this.manager.writeInternal(same(event3));
expectLastCall();
- this.manager.disconnectInternal();
+ this.manager.shutdownInternal();
expectLastCall();
replay(this.manager);
- this.manager.disconnect();
+ this.manager.shutdown();
}
}
Modified: logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManagerTest.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManagerTest.java?rev=1565858&r1=1565857&r2=1565858&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManagerTest.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/nosql/NoSQLDatabaseManagerTest.java Fri Feb 7 23:55:45 2014
@@ -69,7 +69,7 @@ public class NoSQLDatabaseManagerTest {
expect(this.provider.getConnection()).andReturn(this.connection);
replay(this.provider, this.connection);
- manager.connectInternal();
+ manager.startupInternal();
verify(this.provider, this.connection);
reset(this.provider, this.connection);
@@ -78,7 +78,7 @@ public class NoSQLDatabaseManagerTest {
expectLastCall();
replay(this.provider, this.connection);
- manager.disconnectInternal();
+ manager.shutdownInternal();
} finally {
try {
manager.release();
@@ -126,7 +126,7 @@ public class NoSQLDatabaseManagerTest {
final NoSQLDatabaseManager<?> manager = NoSQLDatabaseManager.getNoSQLDatabaseManager("name", 0, this.provider);
try {
- manager.connect();
+ manager.startup();
verify(this.provider, this.connection);
reset(this.provider, this.connection);
@@ -165,7 +165,7 @@ public class NoSQLDatabaseManagerTest {
final NoSQLDatabaseManager<?> manager = NoSQLDatabaseManager.getNoSQLDatabaseManager("name", 0, this.provider);
try {
- manager.connect();
+ manager.startup();
verify(this.provider, this.connection);
reset(this.provider, this.connection);
@@ -251,7 +251,7 @@ public class NoSQLDatabaseManagerTest {
final NoSQLDatabaseManager<?> manager = NoSQLDatabaseManager.getNoSQLDatabaseManager("name", 0, this.provider);
try {
- manager.connect();
+ manager.startup();
verify(this.provider, this.connection);
reset(this.provider, this.connection);
@@ -385,7 +385,7 @@ public class NoSQLDatabaseManagerTest {
final NoSQLDatabaseManager<?> manager = NoSQLDatabaseManager.getNoSQLDatabaseManager("name", 0, this.provider);
try {
- manager.connect();
+ manager.startup();
verify(this.provider, this.connection);
reset(this.provider, this.connection);