Request for review: 6941923: RFE: Handling large log files produced by long running Java Applications

[yumin.qi at oracle.com]

  Poonam,

   thanks.

On 4/13/2011 7:16 PM, Poonam Bajaj wrote:
> Hello Yumin,
>
> Comments inline:
>
> globals.hpp:
>
> 2337   product(uintx, MaxGCLogFileNumbers, 1,                                    \
> 2338           "Maximum number of gclog file roration "                          \
> 2339           "Default rotate in 1 file")                                       \
> 2340                                                                             \
> 2341   product(uintx, GCLogFileSize, 10*M,                                       \
> 2342           "Default log size 10 Megabytes "                                  \
> 2343           "only used when UseGCLogFileRotation set")                        \
> 2344
>
> Here I agree with Jesper that the option names and comments need to be changed for clear understanding. Some suggestons
>   oMaxGCLogFileNumbers can be named asMaxGCLogFiles orMaxGCLogNumberOfFiles.
>   o line 2338 can be worded as"Maximum number of rotating gc log files. Deafult is 1 "
>   o line 2343 should be"only used when UseGCLogFileRotation is set")
>
changed as pointed by Jesper.
>
> arguments.cpp:
> +       if (_gc_log_filename == NULL) {
> +         jio_fprintf(defaultStream::error_stream(),
> +                     "-XX:+UseGCLogRotaion must be with -Xloggc:filename in front\n");
>
> should be:"-XX:+UseGCLogFileRotationmust be used with -Xloggc:<filename>\n"
>
>
Good catch.
> osstream.cpp:
>
>   835     if (UseGCLogFileRotation) {
>   836       gclogStream* rot_tty = new(ResourceObj::C_HEAP)
>   837                                   gclogStream();
>   838       if (rot_tty->is_open()) {
>   839         // now we update the time stamp of the GC log to be synced up with tty.
>   840         rot_tty->time_stamp().update_to(tty->time_stamp().ticks());
>   841       }
>   842       gclog_or_tty = rot_tty;
>   843     }
>   844     else {
>   845       fileStream* gc_tty  = new(ResourceObj::C_HEAP)
>   846                                  fileStream(Arguments::gc_log_filename());
>   847       if (gc_tty->is_open()) {
>   848         // now we update the time stamp of the GC log to be synced up with tty.
>   849         gc_tty->time_stamp().update_to(tty->time_stamp().ticks());
>
>
>   850       }
>   851       gclog_or_tty = gc_tty;
>   852     }
>   853   }
>
>   
>  o here, we are refering to log file so the variable names should be 
> rot_gclog and gclog respectively.
>  o why is the line at 851 out of 'if' block ? gclog_or_tty should be 
> set to gclog file only if it is open and the same holds for the 
> rotating gc log file.
>
I think the code here is if file already open, put a time stamp or just 
assign gclog_or_tty a choice. I may change the code using same 
fileStream --- doing modification now. Since the flag will be an 
external flag, so it will respond to the change request from outside tool.
>
> Thanks,
> Poonam
>
>
>
> On 4/13/2011 12:11 AM, yumin.qi at oracle.com wrote:
>> http://cr.openjdk.java.net/~minqi/6941923/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Eminqi/6941923/webrev.00/>
>>
>> Summary:
>>
>> This is a RFE request for having a GC log rotation to prevent Java 
>> application from over flooding disk with GC output running for long 
>> time.
>> In the implementation, supply three JVM options
>> 1) -XX:+UseGCLogFileRotation   must be used with -Xloggc:file
>> 2) -XX:MaxGCLogFileNumbers=  set limit of rotation file numbers, 
>> default to 1, maximum set to 1024.
>> 3) -XX:GCLogFileSize=  can be configured by user how big the file 
>> size should be. Default to 10M. Minimum set to 512K if given from 
>> option is less than 512K.
>>
>> If MaxGCLogFileNumbers=1, rotating output in same file, i.e write 
>> from beginning of the file when reach cap of the file; with 
>> MaxGCLogFileNumbers > 1 rotating files sequentially after reach cap 
>> in file, file.1, file.2, ..., file.<MaxGCLogFileNumbers-1> then back 
>> to file, file.1, ...
>> Check if rotation needed at safepoint ending.
>>
>> Tested with multiple GC choices.
>>
>> Thanks
>> Yumin
>>
>>
>>
>
> -- 
> Best regards, Poonam
>
> Sun, an Oracle company
> Sun, an Oracle Company
> Poonam Bajaj | Staff Engineer
> Phone: +66937451 <tel:+66937451> | Mobile: +9844511366 <tel:+9844511366>
> JVM Sustaining Engineering
> | Bangalore
> Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
> developing practices and products that help protect the environment
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20110414/26162477/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 2088 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20110414/26162477/attachment.gif 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 356 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20110414/26162477/attachment-0001.gif