tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Hanik - Dev Lists <devli...@hanik.com>
Subject Re: svn commit: r898745 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/juli/FileHandler.java webapps/docs/logging.xml
Date Wed, 13 Jan 2010 16:44:33 GMT
On 01/13/2010 09:24 AM, Konstantin Kolinko wrote:
> 2010/1/13 Filip Hanik - Dev Lists<devlists@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


Mime
View raw message