You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by vi...@apache.org on 2017/06/16 19:17:39 UTC
svn commit: r1798977 - in /tomcat/trunk: conf/logging.properties
java/org/apache/juli/AsyncFileHandler.java
java/org/apache/juli/FileHandler.java
test/org/apache/juli/TestFileHandler.java webapps/docs/changelog.xml
webapps/docs/logging.xml
Author: violetagg
Date: Fri Jun 16 19:17:39 2017
New Revision: 1798977
URL: http://svn.apache.org/viewvc?rev=1798977&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
Add a new JULI FileHandler configuration for specifying the maximum number of days to keep the log files. By default the log files will be kept 90 days as configured in logging.properties.
Added:
tomcat/trunk/test/org/apache/juli/TestFileHandler.java (with props)
Modified:
tomcat/trunk/conf/logging.properties
tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
tomcat/trunk/java/org/apache/juli/FileHandler.java
tomcat/trunk/webapps/docs/changelog.xml
tomcat/trunk/webapps/docs/logging.xml
Modified: tomcat/trunk/conf/logging.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/logging.properties?rev=1798977&r1=1798976&r2=1798977&view=diff
==============================================================================
--- tomcat/trunk/conf/logging.properties (original)
+++ tomcat/trunk/conf/logging.properties Fri Jun 16 19:17:39 2017
@@ -25,18 +25,22 @@ handlers = 1catalina.org.apache.juli.Asy
1catalina.org.apache.juli.AsyncFileHandler.level = FINE
1catalina.org.apache.juli.AsyncFileHandler.directory = ${catalina.base}/logs
1catalina.org.apache.juli.AsyncFileHandler.prefix = catalina.
+1catalina.org.apache.juli.AsyncFileHandler.maxDays = 90
2localhost.org.apache.juli.AsyncFileHandler.level = FINE
2localhost.org.apache.juli.AsyncFileHandler.directory = ${catalina.base}/logs
2localhost.org.apache.juli.AsyncFileHandler.prefix = localhost.
+2localhost.org.apache.juli.AsyncFileHandler.maxDays = 90
3manager.org.apache.juli.AsyncFileHandler.level = FINE
3manager.org.apache.juli.AsyncFileHandler.directory = ${catalina.base}/logs
3manager.org.apache.juli.AsyncFileHandler.prefix = manager.
+3manager.org.apache.juli.AsyncFileHandler.maxDays = 90
4host-manager.org.apache.juli.AsyncFileHandler.level = FINE
4host-manager.org.apache.juli.AsyncFileHandler.directory = ${catalina.base}/logs
4host-manager.org.apache.juli.AsyncFileHandler.prefix = host-manager.
+4host-manager.org.apache.juli.AsyncFileHandler.maxDays = 90
java.util.logging.ConsoleHandler.level = FINE
java.util.logging.ConsoleHandler.formatter = org.apache.juli.OneLineFormatter
Modified: tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java (original)
+++ tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java Fri Jun 16 19:17:39 2017
@@ -71,11 +71,15 @@ public class AsyncFileHandler extends Fi
protected volatile boolean closed = false;
public AsyncFileHandler() {
- this(null, null, null);
+ this(null, null, null, DEFAULT_MAX_DAYS);
}
public AsyncFileHandler(String directory, String prefix, String suffix) {
- super(directory, prefix, suffix);
+ this(directory, prefix, suffix, DEFAULT_MAX_DAYS);
+ }
+
+ public AsyncFileHandler(String directory, String prefix, String suffix, int maxDays) {
+ super(directory, prefix, suffix, maxDays);
open();
}
Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
+++ tomcat/trunk/java/org/apache/juli/FileHandler.java Fri Jun 16 19:17:39 2017
@@ -26,7 +26,16 @@ import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.sql.Timestamp;
+import java.time.DateTimeException;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.ErrorManager;
@@ -36,6 +45,7 @@ import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogManager;
import java.util.logging.LogRecord;
+import java.util.regex.Pattern;
/**
* Implementation of <b>Handler</b> that appends log messages to a file
@@ -74,24 +84,37 @@ import java.util.logging.LogRecord;
* <li><code>formatter</code> - The <code>java.util.logging.Formatter</code>
* implementation class name for this Handler. Default value:
* <code>java.util.logging.SimpleFormatter</code></li>
+ * <li><code>maxDays</code> - The maximum number of days to keep the log
+ * files. If the specified value is <code><=0</code> then the log files
+ * will be kept on the file system forever, otherwise they will be kept the
+ * specified maximum days. Default value: <code>-1</code>.</li>
* </ul>
*/
public class FileHandler extends Handler {
+ public static final int DEFAULT_MAX_DAYS = -1;
+
+ private static final ExecutorService DELETE_FILES_SERVICE = Executors.newSingleThreadExecutor();
// ------------------------------------------------------------ Constructor
public FileHandler() {
- this(null, null, null);
+ this(null, null, null, DEFAULT_MAX_DAYS);
}
public FileHandler(String directory, String prefix, String suffix) {
+ this(directory, prefix, suffix, DEFAULT_MAX_DAYS);
+ }
+
+ public FileHandler(String directory, String prefix, String suffix, int maxDays) {
this.directory = directory;
this.prefix = prefix;
this.suffix = suffix;
+ this.maxDays = maxDays;
configure();
openWriter();
+ clean();
}
@@ -124,12 +147,18 @@ public class FileHandler extends Handler
/**
- * Determines whether the logfile is rotatable
+ * Determines whether the log file is rotatable
*/
private boolean rotatable = true;
/**
+ * Maximum number of days to keep the log files
+ */
+ private int maxDays = DEFAULT_MAX_DAYS;
+
+
+ /**
* The PrintWriter to which we are currently logging, if any.
*/
private volatile PrintWriter writer = null;
@@ -147,6 +176,13 @@ public class FileHandler extends Handler
private int bufferSize = -1;
+ /**
+ * Represents a file name pattern of type {prefix}{date}{suffix}.
+ * The date is YYYY-MM-DD
+ */
+ private Pattern pattern;
+
+
// --------------------------------------------------------- Public Methods
@@ -179,6 +215,7 @@ public class FileHandler extends Handler
closeWriter();
date = tsDate;
openWriter();
+ clean();
}
} finally {
// Downgrade to read-lock. This ensures the writer remains valid
@@ -291,6 +328,16 @@ public class FileHandler extends Handler
if (suffix == null) {
suffix = getProperty(className + ".suffix", ".log");
}
+ pattern = Pattern.compile("^(" + Pattern.quote(prefix) + ")\\d{4}-\\d{1,2}-\\d{1,2}("
+ + Pattern.quote(suffix) + ")$");
+ String sMaxDays = getProperty(className + ".maxDays", String.valueOf(DEFAULT_MAX_DAYS));
+ if (maxDays <= 0) {
+ try {
+ maxDays = Integer.parseInt(sMaxDays);
+ } catch (NumberFormatException ignore) {
+ // no-op
+ }
+ }
String sBufferSize = getProperty(className + ".bufferSize", String.valueOf(bufferSize));
try {
bufferSize = Integer.parseInt(sBufferSize);
@@ -408,4 +455,47 @@ public class FileHandler extends Handler
writerLock.writeLock().unlock();
}
}
+
+ private void clean() {
+ if (maxDays <= 0) {
+ return;
+ }
+ DELETE_FILES_SERVICE.submit(() -> {
+ try (DirectoryStream<Path> files = streamFilesForDelete()) {
+ for (Path file : files) {
+ Files.delete(file);
+ }
+ } catch (IOException e) {
+ reportError("Unable to delete log files older than [" + maxDays + "] days", null,
+ ErrorManager.GENERIC_FAILURE);
+ }
+ });
+ }
+
+ private DirectoryStream<Path> streamFilesForDelete() throws IOException {
+ LocalDate maxDaysOffset = LocalDate.now().minus(maxDays, ChronoUnit.DAYS);
+ return Files.newDirectoryStream(new File(directory).toPath(), path -> {
+ boolean result = false;
+ String date = obtainDateFromPath(path);
+ if (date != null) {
+ try {
+ LocalDate dateFromFile = LocalDate.from(DateTimeFormatter.ISO_LOCAL_DATE.parse(date));
+ result = dateFromFile.isBefore(maxDaysOffset);
+ } catch (DateTimeException e) {
+ // no-op
+ }
+ }
+ return result;
+ });
+ }
+
+ private String obtainDateFromPath(Path path) {
+ String date = path.getFileName().toString();
+ if (pattern.matcher(date).matches()) {
+ date = date.substring(prefix.length());
+ return date.substring(0, date.length() - suffix.length());
+ } else {
+ return null;
+ }
+ }
}
Added: tomcat/trunk/test/org/apache/juli/TestFileHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/juli/TestFileHandler.java?rev=1798977&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/juli/TestFileHandler.java (added)
+++ tomcat/trunk/test/org/apache/juli/TestFileHandler.java Fri Jun 16 19:17:39 2017
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.juli;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestFileHandler {
+
+ private static final String PREFIX_1 = "localhost.";
+ private static final String PREFIX_2 = "test.";
+ private static final String PREFIX_3 = "";
+ private static final String PREFIX_4 = "localhost1";
+ private static final String SUFIX_1 = ".log";
+ private static final String SUFIX_2 = ".txt";
+
+ private File logsDir;
+
+ @Before
+ public void setUp() throws Exception {
+ File logsBase = new File(System.getProperty("tomcat.test.temp", "output/tmp"));
+ if (!logsBase.mkdirs() && !logsBase.isDirectory()) {
+ fail("Unable to create logs directory.");
+ }
+ Path logsBasePath = FileSystems.getDefault().getPath(logsBase.getAbsolutePath());
+ logsDir = Files.createTempDirectory(logsBasePath, "test").toFile();
+
+ generateLogFiles(logsDir, PREFIX_1, SUFIX_2, 3);
+ generateLogFiles(logsDir, PREFIX_2, SUFIX_1, 3);
+ generateLogFiles(logsDir, PREFIX_3, SUFIX_1, 3);
+ generateLogFiles(logsDir, PREFIX_4, SUFIX_1, 3);
+
+ String date = LocalDateTime.now().minusDays(3).toString();
+ File file = new File(logsDir, PREFIX_1 + date + SUFIX_1);
+ file.createNewFile();
+
+ }
+
+ @After
+ public void tearDown() {
+ for (File file : logsDir.listFiles()) {
+ file.delete();
+ }
+ logsDir.delete();
+ }
+
+ @SuppressWarnings("unused")
+ @Test
+ public void testCleanOnInitOneHandler() throws Exception {
+ generateLogFiles(logsDir, PREFIX_1, SUFIX_1, 3);
+
+ FileHandler handler = new FileHandler(logsDir.getAbsolutePath(), PREFIX_1, SUFIX_1, 2);
+
+ Thread.sleep(1000);
+
+ assertTrue(logsDir.list().length == 16);
+ }
+
+ @SuppressWarnings("unused")
+ @Test
+ public void testCleanOnInitMultipleHandlers() throws Exception {
+ generateLogFiles(logsDir, PREFIX_1, SUFIX_1, 3);
+
+ FileHandler handler1 = new FileHandler(logsDir.getAbsolutePath(), PREFIX_1, SUFIX_1, 2);
+ FileHandler handler2 = new FileHandler(logsDir.getAbsolutePath(), PREFIX_1, SUFIX_2, 2);
+ FileHandler handler3 = new FileHandler(logsDir.getAbsolutePath(), PREFIX_2, SUFIX_1, 2);
+ FileHandler handler4 = new FileHandler(logsDir.getAbsolutePath(), PREFIX_3, SUFIX_1, 2);
+
+ Thread.sleep(1000);
+
+ assertTrue(logsDir.list().length == 16);
+ }
+
+ @SuppressWarnings("unused")
+ @Test
+ public void testCleanDisabled() throws Exception {
+ generateLogFiles(logsDir, PREFIX_1, SUFIX_1, 3);
+
+ FileHandler handler = new FileHandler(logsDir.getAbsolutePath(), PREFIX_1, SUFIX_1, -1);
+
+ Thread.sleep(1000);
+
+ assertTrue(logsDir.list().length == 17);
+ }
+
+ private void generateLogFiles(File dir, String prefix, String sufix, int amount)
+ throws IOException {
+ for (int i = 0; i < amount; i++) {
+ String date = LocalDate.now().minusDays(i + 1).toString().substring(0, 10);
+ File file = new File(dir, prefix + date + sufix);
+ file.createNewFile();
+ }
+ }
+}
Propchange: tomcat/trunk/test/org/apache/juli/TestFileHandler.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1798977&r1=1798976&r2=1798977&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Jun 16 19:17:39 2017
@@ -66,6 +66,12 @@
<bug>61101</bug>: CORS filter should set Vary header in response.
Submitted by Rick Riemer. (remm)
</fix>
+ <add>
+ <bug>61105</bug>: Add a new JULI FileHandler configuration for
+ specifying the maximum number of days to keep the log files. By default
+ the log files will be kept 90 days as configured in
+ <code>logging.properties</code>. (violetagg)
+ </add>
<update>
Update the Servlet 4.0 implementation to add support for setting
trailer fields for HTTP responses. (markt)
Modified: tomcat/trunk/webapps/docs/logging.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/logging.xml?rev=1798977&r1=1798976&r2=1798977&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/logging.xml (original)
+++ tomcat/trunk/webapps/docs/logging.xml Fri Jun 16 19:17:39 2017
@@ -287,6 +287,12 @@ java.util.logging.ConsoleHandler.level=A
boolean value.</li>
<li>The root logger can define its set of handlers using the
<code>.handlers</code> property.</li>
+ <li> By default the log files will be kept on the file system
+ <code>90</code>code> days. This may be changed per handler using the
+ <code>handlerName.maxDays</code> property. If the specified value for the
+ property is <code><=0</code> then the log files will be kept on the
+ file system forever, otherwise they will be kept the specified maximum
+ days.</li>
</ul>
<p>
There are several additional implementation classes, that can be used
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1798977 - in /tomcat/trunk: conf/logging.properties
java/org/apache/juli/AsyncFileHandler.java java/org/apache/juli/FileHandler.java
test/org/apache/juli/TestFileHandler.java webapps/docs/changelog.xml webapps/docs/logging.xml
Posted by Violeta Georgieva <vi...@apache.org>.
Hi,
2017-06-27 22:59 GMT+03:00 Rainer Jung <ra...@kippdata.de>:
>
> Am 27.06.2017 um 21:41 schrieb Rainer Jung:
>>
>> Hi Violeta,
>>
>> Am 16.06.2017 um 21:17 schrieb violetagg@apache.org:
>>>
>>> Author: violetagg
>>> Date: Fri Jun 16 19:17:39 2017
>>> New Revision: 1798977
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1798977&view=rev
>>> Log:
>>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
>>> Add a new JULI FileHandler configuration for specifying the maximum
>>> number of days to keep the log files. By default the log files will be
>>> kept 90 days as configured in logging.properties.
>>>
>>> Added:
>>> tomcat/trunk/test/org/apache/juli/TestFileHandler.java (with
props)
>>> Modified:
>>> tomcat/trunk/conf/logging.properties
>>> tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
>>> tomcat/trunk/java/org/apache/juli/FileHandler.java
>>> tomcat/trunk/webapps/docs/changelog.xml
>>> tomcat/trunk/webapps/docs/logging.xml
>>
>>
>> ...
>>
>>> Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
>>> URL:
>>>
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff
>>>
>>>
==============================================================================
>>>
>>> --- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
>>> +++ tomcat/trunk/java/org/apache/juli/FileHandler.java Fri Jun 16
>>> 19:17:39 2017
>>
>> ...
>>
>>> @@ -74,24 +84,37 @@ import java.util.logging.LogRecord;
>>> * <li><code>formatter</code> - The
>>> <code>java.util.logging.Formatter</code>
>>> * implementation class name for this Handler. Default value:
>>> * <code>java.util.logging.SimpleFormatter</code></li>
>>> + * <li><code>maxDays</code> - The maximum number of days to keep
>>> the log
>>> + * files. If the specified value is <code><=0</code> then the
>>> log files
>>> + * will be kept on the file system forever, otherwise they will be
>>> kept the
>>> + * specified maximum days. Default value: <code>-1</code>.</li>
>>> * </ul>
>>> */
>>> public class FileHandler extends Handler {
>>> + public static final int DEFAULT_MAX_DAYS = -1;
>>> +
>>> + private static final ExecutorService DELETE_FILES_SERVICE =
>>> Executors.newSingleThreadExecutor();
>>
>>
>> When testing the M22 release I noticed that a tomcat process was
>> leftover that didn't want to shut down. I checked and I could easily
>> reproduce by starting with startup.sh and then stopping with shutdown.sh
>> (but not using kill). IMHO it didn't shut down, because according to a
>> thread dump it had a single non-daemon thread still running (named
>> "pool-1-thread-1"). Since we typically give more specific names to all
>> threads we create I instrumented the Thread class to find out the
>> creator of that thread and noticed, that it was created here:
>>
>> at java.lang.Thread.<init>(Thread.java:677)
>> at
>>
java.util.concurrent.Executors$DefaultThreadFactory.newThread(Executors.java:613)
>>
>> at
>>
java.util.concurrent.ThreadPoolExecutor$Worker.<init>(ThreadPoolExecutor.java:612)
>>
>> at
>>
java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:925)
>>
>> at
>>
java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1357)
>>
>> at
>>
java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112)
>>
>> at
>>
java.util.concurrent.Executors$DelegatedExecutorService.submit(Executors.java:678)
>>
>> at org.apache.juli.FileHandler.clean(FileHandler.java:463)
>> at org.apache.juli.FileHandler.<init>(FileHandler.java:117)
>> at
>> org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:82)
>> at
>> org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:74)
>>
>> So it is the thread belonging to the above "ExecutorService
>> DELETE_FILES_SERVICE". I could not see, how that thread would ever get
>> stopped, so two remarks:
>>
>> - in order to make sure TC can shut down without problem we either need
>> to stop that thread by ourselves during TC shutdown or mark it as a
>> daemon thread. I guess the latter is preferred.
>>
>> - we should give the created thread a specific name according to its
>> typical task so that it can be identified in any thread dump. That
>> should be doable by the same ThreadFactory.
>>
>> So we need to pass a ThreadFactory to the newSingleThreadExecutor() call
>> (this possibility already exists in the Executors class). To keep juli
>> independent from the other TC classes, we probably need to code the
>> factory inside juli, but we can borrow code from the small class
>> org.apache.tomcat.util.threads.TaskThreadFactory or use code from there
>> to extend the result of Executors.defaultThreadFactory() or
>> Executors.privilegedThreadFactory().
>
>
> I should add: I only observed it for TC 9, because there the feature is
active by default due to the updated juli config file, so the submit to the
executor happens (attribute maxDays in logging.properties).
>
> On all other branches the feature is off by default and although the
executor gets created, I do not see the thread which probably means it gets
created lazily during the first submit. That submit never happens with the
default config, but would once someone actually tries to use the feature.
>
> So of course we should fix on all branches and the feature is somewhat
broken on all branches currently, not only on TC 9.
>
Thanks for the review.
I committed a change that addresses the issues.
In addition I spotted that we need additional permission in order to be
able to delete file when running under Security Manager.
Regards,
Violeta
>
> Regards,
>
> Rainer
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: svn commit: r1798977 - in /tomcat/trunk: conf/logging.properties
java/org/apache/juli/AsyncFileHandler.java
java/org/apache/juli/FileHandler.java
test/org/apache/juli/TestFileHandler.java webapps/docs/changelog.xml
webapps/docs/logging.xml
Posted by Rainer Jung <ra...@kippdata.de>.
Am 27.06.2017 um 21:41 schrieb Rainer Jung:
> Hi Violeta,
>
> Am 16.06.2017 um 21:17 schrieb violetagg@apache.org:
>> Author: violetagg
>> Date: Fri Jun 16 19:17:39 2017
>> New Revision: 1798977
>>
>> URL: http://svn.apache.org/viewvc?rev=1798977&view=rev
>> Log:
>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
>> Add a new JULI FileHandler configuration for specifying the maximum
>> number of days to keep the log files. By default the log files will be
>> kept 90 days as configured in logging.properties.
>>
>> Added:
>> tomcat/trunk/test/org/apache/juli/TestFileHandler.java (with props)
>> Modified:
>> tomcat/trunk/conf/logging.properties
>> tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
>> tomcat/trunk/java/org/apache/juli/FileHandler.java
>> tomcat/trunk/webapps/docs/changelog.xml
>> tomcat/trunk/webapps/docs/logging.xml
>
> ...
>
>> Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
>> URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff
>>
>> ==============================================================================
>>
>> --- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
>> +++ tomcat/trunk/java/org/apache/juli/FileHandler.java Fri Jun 16
>> 19:17:39 2017
> ...
>
>> @@ -74,24 +84,37 @@ import java.util.logging.LogRecord;
>> * <li><code>formatter</code> - The
>> <code>java.util.logging.Formatter</code>
>> * implementation class name for this Handler. Default value:
>> * <code>java.util.logging.SimpleFormatter</code></li>
>> + * <li><code>maxDays</code> - The maximum number of days to keep
>> the log
>> + * files. If the specified value is <code><=0</code> then the
>> log files
>> + * will be kept on the file system forever, otherwise they will be
>> kept the
>> + * specified maximum days. Default value: <code>-1</code>.</li>
>> * </ul>
>> */
>> public class FileHandler extends Handler {
>> + public static final int DEFAULT_MAX_DAYS = -1;
>> +
>> + private static final ExecutorService DELETE_FILES_SERVICE =
>> Executors.newSingleThreadExecutor();
>
> When testing the M22 release I noticed that a tomcat process was
> leftover that didn't want to shut down. I checked and I could easily
> reproduce by starting with startup.sh and then stopping with shutdown.sh
> (but not using kill). IMHO it didn't shut down, because according to a
> thread dump it had a single non-daemon thread still running (named
> "pool-1-thread-1"). Since we typically give more specific names to all
> threads we create I instrumented the Thread class to find out the
> creator of that thread and noticed, that it was created here:
>
> at java.lang.Thread.<init>(Thread.java:677)
> at
> java.util.concurrent.Executors$DefaultThreadFactory.newThread(Executors.java:613)
>
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.<init>(ThreadPoolExecutor.java:612)
>
> at
> java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:925)
>
> at
> java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1357)
>
> at
> java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112)
>
> at
> java.util.concurrent.Executors$DelegatedExecutorService.submit(Executors.java:678)
>
> at org.apache.juli.FileHandler.clean(FileHandler.java:463)
> at org.apache.juli.FileHandler.<init>(FileHandler.java:117)
> at
> org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:82)
> at
> org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:74)
>
> So it is the thread belonging to the above "ExecutorService
> DELETE_FILES_SERVICE". I could not see, how that thread would ever get
> stopped, so two remarks:
>
> - in order to make sure TC can shut down without problem we either need
> to stop that thread by ourselves during TC shutdown or mark it as a
> daemon thread. I guess the latter is preferred.
>
> - we should give the created thread a specific name according to its
> typical task so that it can be identified in any thread dump. That
> should be doable by the same ThreadFactory.
>
> So we need to pass a ThreadFactory to the newSingleThreadExecutor() call
> (this possibility already exists in the Executors class). To keep juli
> independent from the other TC classes, we probably need to code the
> factory inside juli, but we can borrow code from the small class
> org.apache.tomcat.util.threads.TaskThreadFactory or use code from there
> to extend the result of Executors.defaultThreadFactory() or
> Executors.privilegedThreadFactory().
I should add: I only observed it for TC 9, because there the feature is
active by default due to the updated juli config file, so the submit to
the executor happens (attribute maxDays in logging.properties).
On all other branches the feature is off by default and although the
executor gets created, I do not see the thread which probably means it
gets created lazily during the first submit. That submit never happens
with the default config, but would once someone actually tries to use
the feature.
So of course we should fix on all branches and the feature is somewhat
broken on all branches currently, not only on TC 9.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1798977 - in /tomcat/trunk: conf/logging.properties
java/org/apache/juli/AsyncFileHandler.java
java/org/apache/juli/FileHandler.java
test/org/apache/juli/TestFileHandler.java webapps/docs/changelog.xml
webapps/docs/logging.xml
Posted by Rainer Jung <ra...@kippdata.de>.
Hi Violeta,
Am 16.06.2017 um 21:17 schrieb violetagg@apache.org:
> Author: violetagg
> Date: Fri Jun 16 19:17:39 2017
> New Revision: 1798977
>
> URL: http://svn.apache.org/viewvc?rev=1798977&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
> Add a new JULI FileHandler configuration for specifying the maximum number of days to keep the log files. By default the log files will be kept 90 days as configured in logging.properties.
>
> Added:
> tomcat/trunk/test/org/apache/juli/TestFileHandler.java (with props)
> Modified:
> tomcat/trunk/conf/logging.properties
> tomcat/trunk/java/org/apache/juli/AsyncFileHandler.java
> tomcat/trunk/java/org/apache/juli/FileHandler.java
> tomcat/trunk/webapps/docs/changelog.xml
> tomcat/trunk/webapps/docs/logging.xml
...
> Modified: tomcat/trunk/java/org/apache/juli/FileHandler.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/juli/FileHandler.java?rev=1798977&r1=1798976&r2=1798977&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/juli/FileHandler.java (original)
> +++ tomcat/trunk/java/org/apache/juli/FileHandler.java Fri Jun 16 19:17:39 2017
...
> @@ -74,24 +84,37 @@ import java.util.logging.LogRecord;
> * <li><code>formatter</code> - The <code>java.util.logging.Formatter</code>
> * implementation class name for this Handler. Default value:
> * <code>java.util.logging.SimpleFormatter</code></li>
> + * <li><code>maxDays</code> - The maximum number of days to keep the log
> + * files. If the specified value is <code><=0</code> then the log files
> + * will be kept on the file system forever, otherwise they will be kept the
> + * specified maximum days. Default value: <code>-1</code>.</li>
> * </ul>
> */
> public class FileHandler extends Handler {
> + public static final int DEFAULT_MAX_DAYS = -1;
> +
> + private static final ExecutorService DELETE_FILES_SERVICE = Executors.newSingleThreadExecutor();
When testing the M22 release I noticed that a tomcat process was
leftover that didn't want to shut down. I checked and I could easily
reproduce by starting with startup.sh and then stopping with shutdown.sh
(but not using kill). IMHO it didn't shut down, because according to a
thread dump it had a single non-daemon thread still running (named
"pool-1-thread-1"). Since we typically give more specific names to all
threads we create I instrumented the Thread class to find out the
creator of that thread and noticed, that it was created here:
at java.lang.Thread.<init>(Thread.java:677)
at
java.util.concurrent.Executors$DefaultThreadFactory.newThread(Executors.java:613)
at
java.util.concurrent.ThreadPoolExecutor$Worker.<init>(ThreadPoolExecutor.java:612)
at
java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:925)
at
java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1357)
at
java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112)
at
java.util.concurrent.Executors$DelegatedExecutorService.submit(Executors.java:678)
at org.apache.juli.FileHandler.clean(FileHandler.java:463)
at org.apache.juli.FileHandler.<init>(FileHandler.java:117)
at
org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:82)
at
org.apache.juli.AsyncFileHandler.<init>(AsyncFileHandler.java:74)
So it is the thread belonging to the above "ExecutorService
DELETE_FILES_SERVICE". I could not see, how that thread would ever get
stopped, so two remarks:
- in order to make sure TC can shut down without problem we either need
to stop that thread by ourselves during TC shutdown or mark it as a
daemon thread. I guess the latter is preferred.
- we should give the created thread a specific name according to its
typical task so that it can be identified in any thread dump. That
should be doable by the same ThreadFactory.
So we need to pass a ThreadFactory to the newSingleThreadExecutor() call
(this possibility already exists in the Executors class). To keep juli
independent from the other TC classes, we probably need to code the
factory inside juli, but we can borrow code from the small class
org.apache.tomcat.util.threads.TaskThreadFactory or use code from there
to extend the result of Executors.defaultThreadFactory() or
Executors.privilegedThreadFactory().
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org