RFR: 7164841: Improvements to the GC log file rotation

[Lois Foltan]

Hi Yumin,

Looks good, a couple of review comments:

    - I like that you pulled out the number 10 into the EXTRACHARLEN 
macro, can you explain the assigned value of 20?

    - Is -Xloggc:stdout or -Xloggc:stderr allowed?  If yes, can 
-XX:+UseGCLogFileRotation be specified at the same time?

    - Also, I am concerned about the introduction of temporary files 
called "<file_name>.<number>.current", why is this necessary?
      This scheme carries with it the additional potential for failure 
points like the ones you added checks for in 
rotatingFileStream::rotate_log().
      During error conditions, spurious <file_name>.<number>.current 
files could be left around that have no meaning or context.

    - You've added checks in rotatingFileStream::rotate_log() for if a 
pre-existing file can not be removed or if the current
       <file_name>.<number>.current can not be renamed.  If either of 
those errors occur, a warning is issued but then the log
       rotation proceeds.  So there could be a loss of -Xloggc 
information in the concurrent progression of rotating files.
       Would a better approach be to not close the existing log file in 
the rotation, proceed to determine if the next log file can be opened
       and if not, issue a warning, turn of UseGCLogFileRotation and 
continue to concatenate -Xloggc info into current file?

Thanks,
Lois


On 8/15/2013 11:35 AM, Yumin Qi wrote:
> Hi,
>
>   Can I have your review for this small changes?
> http://cr.openjdk.java.net/~minqi/7164841/webrev00/ 
> <http://cr.openjdk.java.net/%7Eminqi/7164841/webrev00/>
>
>    This is for a enhancement to add head/tail message to the logging 
> files to assist reading GC output.
>    1. modified prompt message if invalid arguments used for log rotating;
>    2. add time and file name message to log file head/tail.
>    3. for easily identify which log file is current, use file name 
> like <filename>.n.current, after it reaches maximum size, rename it to 
> <filename>.n
>         On Windows, there is no F_OK (existing test) definition, F_OK 
> is defined as "0" and for _access of VC++, it just describes:
>
> modevalue
>
> 	
>
> Checks file for
>
> 00
>
> 	
>
> Existence only
>
> 02
>
> 	
>
> Write-only
>
> 04
>
> 	
>
> Read-only
>
> 06
>
> 	
>
> Read and write
>
>
> http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx
> The definition are consistent with unistd.h.
>
>     Test: JPRT and jtreg.
>
>    Thanks
>    Yumin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130819/9479a089/attachment-0001.html