You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2010/01/13 17:24:32 UTC
Re: svn commit: r898745 - in /tomcat/tc6.0.x/trunk: STATUS.txt
java/org/apache/juli/FileHandler.java webapps/docs/logging.xml
2010/1/13 Filip Hanik - Dev Lists <de...@hanik.com>:
> -1, I would propose this one to be
>
> writer.write(result);
> if (bufferSize < 0)
> flush();
>
>
> Here is why
>
> 1. No synchronized(this) - not sure why we think its needed
Re: synchronized(this)
- writer.write(result);
+ if (bufferSize > 0) {
+ writer.write(result);
+ } else {
+ synchronized (this) {
+ // OutputStreamWriter performs buffering
inside its StreamEncoder,
+ // and so to run without a buffer we have to
flush explicitly
+ writer.write(result);
+ writer.flush();
+ }
+ }
synchronized (this) {} was added so that writer.write() was
immediately followed by writer.flush().
Both of them are internally synchronized(lock).
Omitting synchronized (this) will result in
writer.write()
writer.write()
writer.flush()
writer.flush()
I do not see much harm from that, so I'd agree to remove synchronized(this).
> 2. It allows a setting of bufferSize==0 -> use system default
> 3. bufferSize<0 do a flush of the writer
I doubt, that such feature is needed, though I'll be OK if anyone proposes it.
Should we use system default (internal buffer of OutputStreamWriter's Encoder),
or our default?
If you want, you can propose a patch that implements this feature and
updates /docs/logging.xml accordingly.
By the way:
with logs buffering enabled by default, as it is now, last log entries
are lost when Tomcat is stopped when Tomcat is run as a service (on
Windows, but I suppose jsvc on Unix has the same effect).
There is Mark's patch for that (r898468), but I have doubts
regarding Runtime.getRuntime().addShutdownHook(new Cleaner());
used there.
We could disable log buffering in 6.0.23, to be it the same as in
6.0.20 (were logs were not buffered):
a) by setting bufferSize=0 or -1 explicitly in the default
logging.properties file
b) by changing the default value in o.a.juli.FileHandler to disable buffering.
I am ok with any of a) and b). Any thoughts?
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r898745 - in /tomcat/tc6.0.x/trunk: STATUS.txt
java/org/apache/juli/FileHandler.java webapps/docs/logging.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/1/13 Mark Thomas <ma...@apache.org>:
> On 13/01/2010 16:24, Konstantin Kolinko wrote:
>> There is Mark's patch for that (r898468), but I have doubts
>> regarding Runtime.getRuntime().addShutdownHook(new Cleaner());
>> used there.
>
> Why? It is pretty much identical to the code in the standard LogManager.
>
One thing is that all shutdown hooks start at the same time and run in
parallel, with no precedence relative each other.
It might help to know, what exactly you are observing. (Maybe file a BZ issue?)
One test, using Windows XP, JRE 6u16:
1. catalina.bat run
2. Press Ctrl+C in the console.
Current trunk and current tc6.0.x: terminates immediately. No
"shutting down" log messages neither on the screen, nor in logs. Thus
messages are lost, and the patch does not fix it. There is some delay,
so I think the shutdown process is actually being performed.
Setting
1catalina.org.apache.juli.FileHandler.bufferSize=-1
in logging.properties does not fix it either.
Current tc5.5.x (though don't remember when I compiled it): Shows a
lot of "shutting down" log messages.
With 6.0.20 I see log messages in the console and in catalina.2010-01-14.log:
14.01.2010 4:38:30 org.apache.coyote.http11.Http11Protocol pause
INFO: Pausing Coyote HTTP/1.1 on http-8080
14.01.2010 4:38:31 org.apache.catalina.core.StandardService stop
INFO: Stopping service Catalina
14.01.2010 4:38:32 org.apache.coyote.http11.Http11Protocol destroy
INFO: Stopping Coyote HTTP/1.1 on http-8080
I thought that I'll also see some occurrences of
reportError("FileHandler is closed or not yet initialized
messages from o.a.juli.FileHandler#publish(),
(I remember seeing them several times last month when Ctrl+C'ing a
console on WinXP)
but I have not observed them now, so that is not confirmed.
> * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48421
> Prevent file descriptor leak. Allow a web application's logs to be deleted
> once the application has been stopped.
>(...)
> http://svn.apache.org/viewvc?rev=898468&view=rev
How this (web application's logs) part of the issue is fixed?
Is it that thanks to the change to LogFactory.java? Or
ClassLoaderLogManager.java change is also needed?
>> We could disable log buffering in 6.0.23, to be it the same as in
>> 6.0.20 (were logs were not buffered):
>> a) by setting bufferSize=0 or -1 explicitly in the default
>> logging.properties file
>> b) by changing the default value in o.a.juli.FileHandler to disable buffering.
>>
>> I am ok with any of a) and b). Any thoughts?
>
> Buffered logs are useful. Lets just fix not flushing the buffers on shut
> down.
>
> Mark
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r898745 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/juli/FileHandler.java
webapps/docs/logging.xml
Posted by Mark Thomas <ma...@apache.org>.
On 13/01/2010 16:24, Konstantin Kolinko wrote:
> There is Mark's patch for that (r898468), but I have doubts
> regarding Runtime.getRuntime().addShutdownHook(new Cleaner());
> used there.
Why? It is pretty much identical to the code in the standard LogManager.
> We could disable log buffering in 6.0.23, to be it the same as in
> 6.0.20 (were logs were not buffered):
> a) by setting bufferSize=0 or -1 explicitly in the default
> logging.properties file
> b) by changing the default value in o.a.juli.FileHandler to disable buffering.
>
> I am ok with any of a) and b). Any thoughts?
Buffered logs are useful. Lets just fix not flushing the buffers on shut
down.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r898745 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/juli/FileHandler.java
webapps/docs/logging.xml
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
On 01/13/2010 09:24 AM, Konstantin Kolinko wrote:
> 2010/1/13 Filip Hanik - Dev Lists<de...@hanik.com>:
>
>> -1, I would propose this one to be
>>
>> writer.write(result);
>> if (bufferSize< 0)
>> flush();
>>
>>
>> Here is why
>>
>> 1. No synchronized(this) - not sure why we think its needed
>>
> Re: synchronized(this)
>
> - writer.write(result);
> + if (bufferSize> 0) {
> + writer.write(result);
> + } else {
> + synchronized (this) {
> + // OutputStreamWriter performs buffering
> inside its StreamEncoder,
> + // and so to run without a buffer we have to
> flush explicitly
> + writer.write(result);
> + writer.flush();
> + }
> + }
>
> synchronized (this) {} was added so that writer.write() was
> immediately followed by writer.flush().
>
> Both of them are internally synchronized(lock).
>
> Omitting synchronized (this) will result in
> writer.write()
> writer.write()
> writer.flush()
> writer.flush()
>
> I do not see much harm from that, so I'd agree to remove synchronized(this).
>
>
>
>> 2. It allows a setting of bufferSize==0 -> use system default
>> 3. bufferSize<0 do a flush of the writer
>>
> I doubt, that such feature is needed, though I'll be OK if anyone proposes it.
>
ok, forcing a flush, could have the impact of a forced disk write, that
is really bad. Let the disk do its own caching, and decide when to flush
it out.
> Should we use system default (internal buffer of OutputStreamWriter's Encoder),
> or our default?
>
system default
> If you want, you can propose a patch that implements this feature and
> updates /docs/logging.xml accordingly.
>
will do
>
> By the way:
> with logs buffering enabled by default, as it is now, last log entries
> are lost when Tomcat is stopped when Tomcat is run as a service (on
> Windows, but I suppose jsvc on Unix has the same effect).
>
got it
> There is Mark's patch for that (r898468), but I have doubts
> regarding Runtime.getRuntime().addShutdownHook(new Cleaner());
> used there.
>
> We could disable log buffering in 6.0.23, to be it the same as in
> 6.0.20 (were logs were not buffered):
> a) by setting bufferSize=0 or -1 explicitly in the default
> logging.properties file
> b) by changing the default value in o.a.juli.FileHandler to disable buffering.
>
> I am ok with any of a) and b). Any thoughts?
>
I would propose
a)
b) default value in FileHandler be 0 (system default behavior)
Filip
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org