Repository: logging-log4j2 Updated Branches: refs/heads/master 887e028a1 -> 61f706fc0
[LOG4J2-1501] FileAppender should be able to create files lazily. Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/61f706fc Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/61f706fc Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/61f706fc Branch: refs/heads/master Commit: 61f706fc0a811c3184e941f2e8d1e012b5cbdb39 Parents: 887e028 Author: Gary Gregory <[email protected]> Authored: Mon Aug 8 08:50:11 2016 -0700 Committer: Gary Gregory <[email protected]> Committed: Mon Aug 8 08:50:11 2016 -0700 ---------------------------------------------------------------------- .../log4j/core/appender/FileAppender.java | 79 +++++- .../log4j/core/appender/FileManager.java | 79 ++++-- .../core/appender/OutputStreamManager.java | 59 +++- .../log4j/core/appender/FileAppenderTest.java | 270 +++++++++++-------- .../log4j/test/appender/InMemoryAppender.java | 7 +- src/changes/changes.xml | 3 + 6 files changed, 346 insertions(+), 151 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java index 90c088e..a69758b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java @@ -39,7 +39,7 @@ import org.apache.logging.log4j.core.util.Integers; @Plugin(name = "File", category = "Core", elementType = "appender", printObject = true) public final class FileAppender extends AbstractOutputStreamAppender<FileManager> { - private static final int DEFAULT_BUFFER_SIZE = 8192; + static final int DEFAULT_BUFFER_SIZE = 8192; private final String fileName; private final Advertiser advertiser; private Object advertisement; @@ -96,8 +96,9 @@ public final class FileAppender extends AbstractOutputStreamAppender<FileManager * @param advertiseUri The advertised URI which can be used to retrieve the file contents. * @param config The Configuration * @return The FileAppender. + * @deprecated Use {@link #createAppender(String, boolean, boolean, String, String, String, boolean, String, Layout<? extends Serializable>, Filter, String, String, boolean, Configuration)} */ - @PluginFactory + @Deprecated public static FileAppender createAppender( // @formatter:off @PluginAttribute("fileName") final String fileName, @@ -144,8 +145,8 @@ public final class FileAppender extends AbstractOutputStreamAppender<FileManager layout = PatternLayout.createDefaultLayout(); } - final FileManager manager = FileManager.getFileManager(fileName, isAppend, isLocking, isBuffered, advertiseUri, - layout, bufferSize, isFlush); + final FileManager manager = FileManager.getFileManager(fileName, isAppend, isLocking, isBuffered, false, + advertiseUri, layout, bufferSize, isFlush); if (manager == null) { return null; } @@ -153,4 +154,74 @@ public final class FileAppender extends AbstractOutputStreamAppender<FileManager return new FileAppender(name, layout, filter, manager, fileName, ignoreExceptions, !isBuffered || isFlush, isAdvertise ? config.getAdvertiser() : null); } + + /** + * Create a File Appender. + * @param fileName The name and path of the file. + * @param append "True" if the file should be appended to, "false" if it should be overwritten. + * The default is "true". + * @param locking "True" if the file should be locked. The default is "false". + * @param name The name of the Appender. + * @param immediateFlush "true" if the contents should be flushed on every write, "false" otherwise. The default + * is "true". + * @param ignoreExceptions If {@code "true"} (default) exceptions encountered when appending events are logged; otherwise + * they are propagated to the caller. + * @param bufferedIo "true" if I/O should be buffered, "false" otherwise. The default is "true". + * @param bufferSize buffer size for buffered IO (default is 8192). + * @param layout The layout to use to format the event. If no layout is provided the default PatternLayout + * will be used. + * @param filter The filter, if any, to use. + * @param advertise "true" if the appender configuration should be advertised, "false" otherwise. + * @param advertiseUri The advertised URI which can be used to retrieve the file contents. + * @param lazyCreate true if you want to lazy-create the file (a.k.a. on-demand.) + * @param config The Configuration + * @return The FileAppender. + * @since 2.7 + */ + @PluginFactory + public static FileAppender createAppender( + // @formatter:off + @PluginAttribute("fileName") final String fileName, + @PluginAttribute(value = "append", defaultBoolean = true) final boolean append, + @PluginAttribute("locking") final boolean locking, + @PluginAttribute("name") final String name, + @PluginAttribute(value = "immediateFlush", defaultBoolean = true) final boolean immediateFlush, + @PluginAttribute(value = "ignoreExceptions", defaultBoolean = true) final boolean ignoreExceptions, + @PluginAttribute(value = "bufferedIo", defaultBoolean = true) boolean bufferedIo, + @PluginAttribute(value = "bufferSize", defaultInt = DEFAULT_BUFFER_SIZE) final int bufferSize, + @PluginElement("Layout") Layout<? extends Serializable> layout, + @PluginElement("Filter") final Filter filter, + @PluginAttribute("advertise") final boolean advertise, + @PluginAttribute("advertiseUri") final String advertiseUri, + @PluginAttribute("lazyCreate") final boolean lazyCreate, + @PluginConfiguration final Configuration config) { + // @formatter:on + if (locking && bufferedIo) { + LOGGER.warn("Locking and buffering are mutually exclusive. No buffering will occur for {}", fileName); + bufferedIo = false; + } + if (!bufferedIo && bufferSize > 0) { + LOGGER.warn("The bufferSize is set to {} but bufferedIo is not true: {}", bufferSize, bufferedIo); + } + if (name == null) { + LOGGER.error("No name provided for FileAppender"); + return null; + } + if (fileName == null) { + LOGGER.error("No filename provided for FileAppender with name {}", name); + return null; + } + if (layout == null) { + layout = PatternLayout.createDefaultLayout(); + } + + final FileManager manager = FileManager.getFileManager(fileName, append, locking, bufferedIo, lazyCreate, + advertiseUri, layout, bufferSize, immediateFlush); + if (manager == null) { + return null; + } + + return new FileAppender(name, layout, filter, manager, fileName, ignoreExceptions, !bufferedIo || immediateFlush, + advertise ? config.getAdvertiser() : null); + } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java index af10d25..71d152b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java @@ -51,7 +51,11 @@ public class FileManager extends OutputStreamManager { this(fileName, os, append, locking, advertiseURI, layout, writeHeader, ByteBuffer.wrap(new byte[bufferSize])); } - /** @since 2.6 */ + /** + * @deprecated + * @since 2.6 + */ + @Deprecated protected FileManager(final String fileName, final OutputStream os, final boolean append, final boolean locking, final String advertiseURI, final Layout<? extends Serializable> layout, final boolean writeHeader, final ByteBuffer buffer) { @@ -62,12 +66,27 @@ public class FileManager extends OutputStreamManager { this.bufferSize = buffer.capacity(); } + /** + * @throws IOException + * @since 2.7 + */ + protected FileManager(final String fileName, final boolean append, final boolean locking, final boolean lazyCreate, + final String advertiseURI, final Layout<? extends Serializable> layout, final boolean writeHeader, + final ByteBuffer buffer) throws IOException { + super(fileName, lazyCreate, layout, writeHeader, buffer); + this.isAppend = append; + this.isLocking = locking; + this.advertiseURI = advertiseURI; + this.bufferSize = buffer.capacity(); + } + /** * Returns the FileManager. * @param fileName The name of the file to manage. * @param append true if the file should be appended to, false if it should be overwritten. * @param locking true if the file should be locked while writing, false otherwise. * @param bufferedIo true if the contents should be buffered as they are written. + * @param lazyCreate true if you want to lazy-create the file (a.k.a. on-demand.) * @param advertiseUri the URI to use when advertising the file * @param layout The layout * @param bufferSize buffer size for buffered IO @@ -75,31 +94,40 @@ public class FileManager extends OutputStreamManager { * @return A FileManager for the File. */ public static FileManager getFileManager(final String fileName, final boolean append, boolean locking, - final boolean bufferedIo, final String advertiseUri, final Layout<? extends Serializable> layout, - final int bufferSize, final boolean immediateFlush) { + final boolean bufferedIo, boolean lazyCreate, final String advertiseUri, + final Layout<? extends Serializable> layout, final int bufferSize, final boolean immediateFlush) { if (locking && bufferedIo) { locking = false; } - return (FileManager) getManager(fileName, new FactoryData(append, locking, bufferedIo, bufferSize, - immediateFlush, advertiseUri, layout), FACTORY); + return (FileManager) getManager(fileName, + new FactoryData(append, locking, bufferedIo, bufferSize, immediateFlush, lazyCreate, advertiseUri, layout), + FACTORY); } @Override - protected synchronized void write(final byte[] bytes, final int offset, final int length, final boolean immediateFlush) { - + protected OutputStream createOutputStream() throws FileNotFoundException { + return new FileOutputStream(getFileName(), isAppend); + } + + @Override + protected synchronized void write(final byte[] bytes, final int offset, final int length, + final boolean immediateFlush) { if (isLocking) { - final FileChannel channel = ((FileOutputStream) getOutputStream()).getChannel(); - /* - * Lock the whole file. This could be optimized to only lock from the current file position. Note that - * locking may be advisory on some systems and mandatory on others, so locking just from the current - * position would allow reading on systems where locking is mandatory. Also, Java 6 will throw an exception - * if the region of the file is already locked by another FileChannel in the same JVM. Hopefully, that will - * be avoided since every file should have a single file manager - unless two different files strings are - * configured that somehow map to the same file. - */ - try (final FileLock lock = channel.lock(0, Long.MAX_VALUE, false)) { - super.write(bytes, offset, length, immediateFlush); + try { + @SuppressWarnings("resource") + final FileChannel channel = ((FileOutputStream) getOutputStream()).getChannel(); + /* + * Lock the whole file. This could be optimized to only lock from the current file position. Note that + * locking may be advisory on some systems and mandatory on others, so locking just from the current + * position would allow reading on systems where locking is mandatory. Also, Java 6 will throw an + * exception if the region of the file is already locked by another FileChannel in the same JVM. + * Hopefully, that will be avoided since every file should have a single file manager - unless two + * different files strings are configured that somehow map to the same file. + */ + try (final FileLock lock = channel.lock(0, Long.MAX_VALUE, false)) { + super.write(bytes, offset, length, immediateFlush); + } } catch (final IOException ex) { throw new AppenderLoggingException("Unable to obtain lock on " + getName(), ex); } @@ -162,6 +190,7 @@ public class FileManager extends OutputStreamManager { private final boolean bufferedIO; private final int bufferSize; private final boolean immediateFlush; + private final boolean lazyCreate; private final String advertiseURI; private final Layout<? extends Serializable> layout; @@ -172,15 +201,19 @@ public class FileManager extends OutputStreamManager { * @param bufferedIO Buffering flag. * @param bufferSize Buffer size. * @param immediateFlush flush on every write or not + * @param lazyCreate if you want to lazy-create the file (a.k.a. on-demand.) * @param advertiseURI the URI to use when advertising the file + * @param layout The layout */ public FactoryData(final boolean append, final boolean locking, final boolean bufferedIO, final int bufferSize, - final boolean immediateFlush, final String advertiseURI, final Layout<? extends Serializable> layout) { + final boolean immediateFlush, final boolean lazyCreate, final String advertiseURI, + final Layout<? extends Serializable> layout) { this.append = append; this.locking = locking; this.bufferedIO = bufferedIO; this.bufferSize = bufferSize; this.immediateFlush = immediateFlush; + this.lazyCreate = lazyCreate; this.advertiseURI = advertiseURI; this.layout = layout; } @@ -192,12 +225,11 @@ public class FileManager extends OutputStreamManager { private static class FileManagerFactory implements ManagerFactory<FileManager, FactoryData> { /** - * Create a FileManager. + * Creates a FileManager. * @param name The name of the File. * @param data The FactoryData * @return The FileManager for the File. */ - @SuppressWarnings("resource") @Override public FileManager createManager(final String name, final FactoryData data) { final File file = new File(name); @@ -208,12 +240,11 @@ public class FileManager extends OutputStreamManager { final boolean writeHeader = !data.append || !file.exists(); try { - final FileOutputStream fos = new FileOutputStream(name, data.append); final int actualSize = data.bufferedIO ? data.bufferSize : Constants.ENCODER_BYTE_BUFFER_SIZE; final ByteBuffer buffer = ByteBuffer.wrap(new byte[actualSize]); - return new FileManager(name, fos, data.append, data.locking, data.advertiseURI, data.layout, + return new FileManager(name, data.append, data.locking, data.lazyCreate, data.advertiseURI, data.layout, writeHeader, buffer); - } catch (final FileNotFoundException ex) { + } catch (final IOException ex) { LOGGER.error("FileManager (" + name + ") " + ex, ex); } return null; http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java index 794610c..e707bea 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamManager.java @@ -18,6 +18,7 @@ package org.apache.logging.log4j.core.appender; import java.io.IOException; import java.io.OutputStream; +import java.io.Serializable; import java.nio.ByteBuffer; import java.util.Objects; @@ -37,6 +38,7 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe protected OutputStreamManager(final OutputStream os, final String streamName, final Layout<?> layout, final boolean writeHeader) { + // Can't use new ctor because it throws an exception this(os, streamName, layout, writeHeader, ByteBuffer.wrap(new byte[Constants.ENCODER_BYTE_BUFFER_SIZE])); } @@ -48,7 +50,9 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe * @param writeHeader * @param byteBuffer * @since 2.6 + * @deprecated */ + @Deprecated protected OutputStreamManager(final OutputStream os, final String streamName, final Layout<?> layout, final boolean writeHeader, final ByteBuffer byteBuffer) { super(streamName); @@ -58,7 +62,7 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe final byte[] header = layout.getHeader(); if (header != null) { try { - this.os.write(header, 0, header.length); + getOutputStream().write(header, 0, header.length); } catch (final IOException e) { logError("Unable to write header", e); } @@ -68,6 +72,30 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe } /** + * @param byteBuffer + * @throws IOException + * @since 2.7 + */ + protected OutputStreamManager(final String streamName, final boolean lazyCreate, final Layout<? extends Serializable> layout, + final boolean writeHeader, final ByteBuffer byteBuffer) + throws IOException { + super(streamName); + this.layout = layout; + this.byteBuffer = Objects.requireNonNull(byteBuffer, "byteBuffer"); + this.os = lazyCreate ? null : createOutputStream(); + if (writeHeader && layout != null) { + final byte[] header = layout.getHeader(); + if (header != null) { + try { + getOutputStream().write(header, 0, header.length); + } catch (final IOException e) { + logError("Unable to write header for " + streamName, e); + } + } + } + } + + /** * Creates a Manager. * * @param name The name of the stream to manage. @@ -81,6 +109,11 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe return AbstractManager.getManager(name, factory, data); } + @SuppressWarnings("unused") + protected OutputStream createOutputStream() throws IOException { + throw new IllegalStateException(getClass().getCanonicalName() + " must implement createOutputStream()"); + } + /** * Indicate whether the footer should be skipped or not. * @param skipFooter true if the footer should be skipped. @@ -119,7 +152,10 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe return getCount() > 0; } - protected OutputStream getOutputStream() { + protected OutputStream getOutputStream() throws IOException { + if (os == null) { + os = createOutputStream(); + } return os; } @@ -208,10 +244,9 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe */ protected synchronized void writeToDestination(final byte[] bytes, final int offset, final int length) { try { - os.write(bytes, offset, length); + getOutputStream().write(bytes, offset, length); } catch (final IOException ex) { - final String msg = "Error writing to stream " + getName(); - throw new AppenderLoggingException(msg, ex); + throw new AppenderLoggingException("Error writing to stream " + getName(), ex); } } @@ -220,11 +255,13 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe * @since 2.6 */ protected synchronized void flushDestination() { - try { - os.flush(); - } catch (final IOException ex) { - final String msg = "Error flushing stream " + getName(); - throw new AppenderLoggingException(msg, ex); + final OutputStream stream = os; // access volatile field only once per method + if (stream != null) { + try { + stream.flush(); + } catch (final IOException ex) { + throw new AppenderLoggingException("Error flushing stream " + getName(), ex); + } } } @@ -255,7 +292,7 @@ public class OutputStreamManager extends AbstractManager implements ByteBufferDe protected synchronized void close() { flush(); final OutputStream stream = os; // access volatile field only once per method - if (stream == System.out || stream == System.err) { + if (stream == null || stream == System.out || stream == System.err) { return; } try { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java index 9934d10..760715d 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.InputStreamReader; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -44,117 +45,151 @@ import org.junit.Assert; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; /** - * + * Tests {@link FileAppender}. */ +@RunWith(Parameterized.class) public class FileAppenderTest { - private static final String FILENAME = "target/fileAppenderTest.log"; + @Parameters(name = "lazyCreate = {0}") + public static Boolean[] getParameters() { + return new Boolean[] { false, true }; + } + + private static final String FILE_NAME = "target/fileAppenderTest.log"; + private static final Path PATH = Paths.get(FILE_NAME); private static final int THREADS = 2; + public FileAppenderTest(boolean lazyCreate) { + super(); + this.lazyCreate = lazyCreate; + } + + private final boolean lazyCreate; + private final int threadCount = THREADS; + @Rule - public CleanFiles files = new CleanFiles(FILENAME); + public CleanFiles files = new CleanFiles(PATH); @AfterClass public static void cleanupClass() { - assertTrue("Manager for " + FILENAME + " not removed", !AbstractManager.hasManager(FILENAME)); + assertTrue("Manager for " + FILE_NAME + " not removed", !AbstractManager.hasManager(FILE_NAME)); } @Test public void testAppender() throws Exception { - writer(false, 1, "test"); - verifyFile(1); + final int logEventCount = 1; + writer(false, logEventCount, "test", lazyCreate, false); + verifyFile(logEventCount); + } + + @Test + public void testLazyStart() throws Exception { + final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, false, "test", false, false, false, + 1, layout, null, false, null, lazyCreate, null); + try { + Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); + appender.start(); + Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); + } finally { + appender.stop(); + } + Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); } @Test public void testSmallestBufferSize() throws Exception { - final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN).build(); - final String bufferSizeStr = "1"; - final FileAppender appender = FileAppender.createAppender(FILENAME, "true", "false", "test", "false", "false", - "false", bufferSizeStr, layout, null, "false", null, null); - appender.start(); - final File file = new File(FILENAME); - assertTrue("Appender did not start", appender.isStarted()); - long curLen = file.length(); - long prevLen = curLen; - assertTrue("File length: " + curLen, curLen == 0); - for (int i = 0; i < 100; ++i) { - final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") // - .setLoggerFqcn(FileAppenderTest.class.getName()).setLevel(Level.INFO) // - .setMessage(new SimpleMessage("Test")).setThreadName(this.getClass().getSimpleName()) // - .setTimeMillis(System.currentTimeMillis()).build(); - try { - appender.append(event); - curLen = file.length(); - assertTrue("File length: " + curLen, curLen > prevLen); - Thread.sleep(25); // Give up control long enough for another thread/process to occasionally do - // something. - } catch (final Exception ex) { - throw ex; + final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, false, "test", false, false, false, + 1, layout, null, false, null, lazyCreate, null); + try { + appender.start(); + final File file = new File(FILE_NAME); + assertTrue("Appender did not start", appender.isStarted()); + Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); + long curLen = file.length(); + long prevLen = curLen; + assertTrue("File length: " + curLen, curLen == 0); + for (int i = 0; i < 100; ++i) { + final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") // + .setLoggerFqcn(FileAppenderTest.class.getName()).setLevel(Level.INFO) // + .setMessage(new SimpleMessage("Test")).setThreadName(this.getClass().getSimpleName()) // + .setTimeMillis(System.currentTimeMillis()).build(); + try { + appender.append(event); + curLen = file.length(); + assertTrue("File length: " + curLen, curLen > prevLen); + // Give up control long enough for another thread/process to occasionally do something. + Thread.sleep(25); + } catch (final Exception ex) { + throw ex; + } + prevLen = curLen; } - prevLen = curLen; + } finally { + appender.stop(); } - appender.stop(); assertFalse("Appender did not stop", appender.isStarted()); } @Test public void testLockingAppender() throws Exception { - writer(true, 1, "test"); - verifyFile(1); + final int logEventCount = 1; + writer(true, logEventCount, "test", lazyCreate, false); + verifyFile(logEventCount); } @Test - public void testMultipleAppenders() throws Exception { - final ExecutorService pool = Executors.newFixedThreadPool(THREADS); - final Exception[] error = new Exception[1]; - final int count = 100; - final Runnable runnable = new FileWriterRunnable(false, count, error); - for (int i = 0; i < THREADS; ++i) { - pool.execute(runnable); + public void testMultipleAppenderThreads() throws Exception { + testMultipleLockingAppenderThreads(false, threadCount); + } + + private void testMultipleLockingAppenderThreads(final boolean lock, int threadCount) + throws InterruptedException, Exception { + final ExecutorService threadPool = Executors.newFixedThreadPool(threadCount); + final Exception[] exceptionRef = new Exception[1]; + final int logEventCount = 100; + final Runnable runnable = new FileWriterRunnable(lock, logEventCount, exceptionRef); + for (int i = 0; i < threadCount; ++i) { + threadPool.execute(runnable); } - pool.shutdown(); - pool.awaitTermination(10, TimeUnit.SECONDS); - if (error[0] != null) { - throw error[0]; + threadPool.shutdown(); + Assert.assertTrue("The thread pool has not shutdown: " + threadPool, + threadPool.awaitTermination(10, TimeUnit.SECONDS)); + if (exceptionRef[0] != null) { + throw exceptionRef[0]; } - verifyFile(THREADS * count); + verifyFile(threadCount * logEventCount); } @Test - public void testMultipleLockedAppenders() throws Exception { - final ExecutorService pool = Executors.newFixedThreadPool(THREADS); - final Exception[] error = new Exception[1]; - final int count = 100; - final Runnable runnable = new FileWriterRunnable(true, count, error); - for (int i = 0; i < THREADS; ++i) { - pool.execute(runnable); - } - pool.shutdown(); - pool.awaitTermination(10, TimeUnit.SECONDS); - if (error[0] != null) { - throw error[0]; - } - verifyFile(THREADS * count); + public void testMultipleLockingAppenders() throws Exception { + testMultipleLockingAppenderThreads(true, threadCount); } @Test @Ignore public void testMultipleVMs() throws Exception { final String classPath = System.getProperty("java.class.path"); - final Integer count = 10; - final int processeCount = 3; - final Process[] processes = new Process[processeCount]; - final ProcessBuilder[] builders = new ProcessBuilder[processeCount]; - for (int index = 0; index < processeCount; ++index) { + final Integer logEventCount = 10; + final int processCount = 3; + final Process[] processes = new Process[processCount]; + final ProcessBuilder[] builders = new ProcessBuilder[processCount]; + for (int index = 0; index < processCount; ++index) { builders[index] = new ProcessBuilder("java", "-cp", classPath, ProcessTest.class.getName(), - "Process " + index, count.toString(), "true"); + "Process " + index, logEventCount.toString(), "true", Boolean.toString(lazyCreate)); } - for (int index = 0; index < processeCount; ++index) { + for (int index = 0; index < processCount; ++index) { processes[index] = builders[index].start(); } - for (int index = 0; index < processeCount; ++index) { + for (int index = 0; index < processCount; ++index) { final Process process = processes[index]; // System.out.println("Process " + index + " exited with " + p.waitFor()); try (final BufferedReader br = new BufferedReader(new InputStreamReader(process.getInputStream()))) { @@ -165,63 +200,75 @@ public class FileAppenderTest { } process.destroy(); } - verifyFile(count * processeCount); + verifyFile(logEventCount * processCount); } - private static void writer(final boolean lock, final int count, final String name) throws Exception { - final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN).build(); - final FileAppender app = FileAppender.createAppender(FILENAME, "true", Boolean.toString(lock), "test", "false", - "false", "false", null, layout, null, "false", null, null); - app.start(); - assertTrue("Appender did not start", app.isStarted()); - Assert.assertTrue(Files.exists(Paths.get(FILENAME))); - for (int i = 0; i < count; ++i) { - final LogEvent event = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") - .setLoggerFqcn(FileAppenderTest.class.getName()).setLevel(Level.INFO) - .setMessage(new SimpleMessage("Test")).setThreadName(name).setTimeMillis(System.currentTimeMillis()) - .build(); - try { - app.append(event); - Thread.sleep(25); // Give up control long enough for another thread/process to occasionally do - // something. - } catch (final Exception ex) { - throw ex; + private static void writer(final boolean lock, final int logEventCount, final String name, boolean lazyCreate, + boolean concurrent) throws Exception { + final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, lock, "test", false, false, false, + FileAppender.DEFAULT_BUFFER_SIZE, layout, null, false, null, lazyCreate, null); + try { + appender.start(); + assertTrue("Appender did not start", appender.isStarted()); + final boolean exists = Files.exists(PATH); + String msg = String.format("concurrent = %s, lazyCreate = %s, file exists = %s", concurrent, lazyCreate, + exists); + // If concurrent the file might have been created (or not.) + // Can't really test lazyCreate && concurrent. + final boolean expectFileCreated = !lazyCreate; + if (concurrent && expectFileCreated) { + Assert.assertTrue(msg, exists); + } else if (expectFileCreated) { + Assert.assertNotEquals(msg, lazyCreate, exists); } + for (int i = 0; i < logEventCount; ++i) { + final LogEvent logEvent = Log4jLogEvent.newBuilder().setLoggerName("TestLogger") + .setLoggerFqcn(FileAppenderTest.class.getName()).setLevel(Level.INFO) + .setMessage(new SimpleMessage("Test")).setThreadName(name) + .setTimeMillis(System.currentTimeMillis()).build(); + try { + appender.append(logEvent); + Thread.sleep(25); // Give up control long enough for another thread/process to occasionally do + // something. + } catch (final Exception ex) { + throw ex; + } + } + } finally { + appender.stop(); } - app.stop(); - assertFalse("Appender did not stop", app.isStarted()); + assertFalse("Appender did not stop", appender.isStarted()); } private void verifyFile(final int count) throws Exception { // String expected = "[\\w]* \\[\\s*\\] INFO TestLogger - Test$"; final String expected = "^\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2},\\d{3} \\[[^\\]]*\\] INFO TestLogger - Test"; final Pattern pattern = Pattern.compile(expected); - final FileInputStream fis = new FileInputStream(FILENAME); - final BufferedReader is = new BufferedReader(new InputStreamReader(fis)); - int counter = 0; - String str = Strings.EMPTY; - while (is.ready()) { - str = is.readLine(); - // System.out.println(str); - ++counter; - final Matcher matcher = pattern.matcher(str); - assertTrue("Bad data: " + str, matcher.matches()); + int lines = 0; + try (final BufferedReader is = new BufferedReader(new InputStreamReader(new FileInputStream(FILE_NAME)))) { + String str = Strings.EMPTY; + while (is.ready()) { + str = is.readLine(); + // System.out.println(str); + ++lines; + final Matcher matcher = pattern.matcher(str); + assertTrue("Unexpected data: " + str, matcher.matches()); + } } - fis.close(); - assertTrue("Incorrect count: was " + counter + " should be " + count, count == counter); - fis.close(); - + Assert.assertEquals(count, lines); } public class FileWriterRunnable implements Runnable { private final boolean lock; - private final int count; - private final Exception[] error; + private final int logEventCount; + private final Exception[] exceptionRef; - public FileWriterRunnable(final boolean lock, final int count, final Exception[] error) { + public FileWriterRunnable(final boolean lock, final int logEventCount, final Exception[] exceptionRef) { this.lock = lock; - this.count = count; - this.error = error; + this.logEventCount = logEventCount; + this.exceptionRef = exceptionRef; } @Override @@ -229,10 +276,9 @@ public class FileAppenderTest { final Thread thread = Thread.currentThread(); try { - writer(lock, count, thread.getName()); - + writer(lock, logEventCount, thread.getName(), lazyCreate, true); } catch (final Exception ex) { - error[0] = ex; + exceptionRef[0] = ex; throw new RuntimeException(ex); } } @@ -256,10 +302,12 @@ public class FileAppenderTest { } final boolean lock = Boolean.parseBoolean(args[2]); + final boolean lazyCreate = Boolean.parseBoolean(args[2]); + // System.out.println("Got arguments " + id + ", " + count + ", " + lock); try { - writer(lock, count, id); + writer(lock, count, id, lazyCreate, true); // thread.sleep(50); } catch (final Exception ex) { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/InMemoryAppender.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/InMemoryAppender.java b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/InMemoryAppender.java index 2d7ccaa..da8b71a 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/InMemoryAppender.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/InMemoryAppender.java @@ -17,6 +17,7 @@ package org.apache.logging.log4j.test.appender; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.Serializable; import org.apache.logging.log4j.core.Layout; @@ -48,7 +49,11 @@ public class InMemoryAppender extends AbstractOutputStreamAppender<InMemoryAppen @Override public String toString() { - return getOutputStream().toString(); + try { + return getOutputStream().toString(); + } catch (IOException e) { + throw new IllegalStateException(e); + } } } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/61f706fc/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 1f1fa85..922aef0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -60,6 +60,9 @@ <action issue="LOG4J2-1313" dev="rpopma" type="fix" due-to="Philipp Knobel"> Properties declared in configuration can now have their value either in the element body or in an attribute named "value". </action> + <action issue="LOG4J2-1501" dev="ggregory" type="add" due-to="Gary Gregory"> + [LOG4J2-1501] FileAppender should be able to create files lazily. + </action> <action issue="LOG4J2-1471" dev="ggregory" type="add" due-to="Gary Gregory"> [PatternLayout] Add an ANSI option to %xThrowable. </action>
