<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<div class="moz-cite-prefix">On 9/4/2013 12:27 PM, Yumin Qi wrote:<br>
</div>
<blockquote cite="mid:52275F68.9080704@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Thanks, Lois<br>
<br>
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
arguments.cpp - <br>
- line # 1867, please add more information on the context in
which this function can be called<br>
<span class="new"><br>
</span></blockquote>
I change the comments to:<br>
<br>
// This function is called for -Xloggc:<filename>, it can be
used<br>
// to check if a given file name(or string) conform to following<br>
// request:<br>
// A valid string only contains "[A-Z][a-z][0-9].-_%[p|t]"<br>
// %p and %t only allowed once.<br>
<br>
Is this OK?<br>
</blockquote>
<br>
Hi Yumin,<br>
Yes, thank you, just a nit, instead of "conform to following
request:" clearer to state "conforms to the following
specification:"<br>
<br>
<blockquote cite="mid:52275F68.9080704@oracle.com" type="cite">
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite"><span
class="new"> - line # 1868, // A valid file name only
contains "[A-Z][a-z][0-9].-_%[p|t]"</span> <br>
I really like the feature that allows a user to tailor the
log file name to include or not include the pid and timestamp.<br>
I think this is a big improvement. However, this does
represent a change over current accepted behavior. By accepting
<br>
only "[A-Z][a-z][0-9] and "." in the file name, a user,
who once specified -Xloggc:foo%.log for example, will now
receive <br>
an error. Or for that matter any special character like
?, *, +. So, I don't know if this is change in behavior is<br>
allowable.<br>
<br>
</blockquote>
I think we can constrain the special chars for file name usage
here --- there will be problem when user upgrade to new version
which contains this changeset so they need to change their scripts
to follow the requirement in this change. <br>
<br>
Wish to get more input for this change.<br>
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
ostream.hpp - looks good, no comments<br>
<br>
ostream.cpp -<br>
- Remove extra lines (#372 & #446) around
extend_file_name() function<br>
<br>
</blockquote>
will<br>
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
- It would be good to have a couple more comments in
extend_file_name(), <br>
like for example, between line #384 and #385 something
like:<br>
// No user specified request for pid & timestamp
in -Xloggc file name.<br>
Between 431 & 432, maybe // Only requested pid <br>
Between 438 & 439, maybe // Only requested timestamp<br>
<br>
</blockquote>
Will add<br>
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
- In the situation where NumberOfGCLogFiles > 1, how does the
very first file opened <br>
in the rotation obtain the additional GC log file header
info you have added? The first <br>
file is opened in the constructor so does not have the
benefit of being opened at line <br>
#647 and then having the header info written to it in line
#658-663.<br>
<br>
- I think the additional GC log file header info would be
beneficial to a user even in the situation<br>
where no UseGCLogFileRotation options are specified on the
command line. A user may<br>
be confused why when just -Xloggc:<filename> is
specified no additional header info is provided.<br>
One possible solution to this and the previous comment is
to add similar code as #658-663 to the<br>
constructor.<br>
<br>
- In rotatingFileStream::rotate_log(), I still have
questions about the error situation where the next file <br>
in the rotation can not be opened. The current file was
closed at line #619, UseGCLogFileRotation is<br>
turned off at line #675, so how will e current file be
reopened to accommodate for the next write in<br>
rotatingFileStream::write()?<br>
<br>
</blockquote>
Good catch. I thought of this, and think rotation will override
that info. Will change to add this when the first time file is
created for both non-rotate and rotate settings.<br>
</blockquote>
My original suggestion of just duplicating lines #658-663 in the
constructor can be improved upon. I think it would be worthwhile to
consider adding a function named something like,
dump_xloggc_header(), for example. This allows for future
additional pertinent information to be included in the header very
easily without having to visit all the different places where the
header info is generated. <br>
<br>
Thanks,<br>
Lois<br>
<blockquote cite="mid:52275F68.9080704@oracle.com" type="cite"> <br>
Thanks<br>
Yumin<br>
<br>
<blockquote cite="mid:5225FB82.9080602@oracle.com" type="cite">
<br>
Thanks,<br>
Lois<br>
<br>
<div class="moz-cite-prefix">On 8/31/2013 2:36 AM, Yumin Qi
wrote:<br>
</div>
<blockquote cite="mid:52218EDE.4060508@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Hi, all<br>
<br>
<br>
With more feedback on the second version, the third version
make following changes:<br>
<br>
1) take parametrized filename after -Xloggc:, the filename
will be forced to follow following rules:<br>
can only contain [A-Z][a-z][0-9].-_%[p|t], any other
character used for file name will be rejected. %p or %t can
only appear once in file name.<br>
example: -Xloggc:test.log-%t-%p OK<br>
example: -Xloggc:test.log-%p-%p FAIL<br>
<br>
2) removed unused rotatingFileStream constructors<br>
<br>
3) log more information at head of rotation file:<br>
vm version, os version, build id etc<br>
memory usage<br>
commandline flags<br>
<br>
Tested on Linux/Windows<br>
<br>
URL: <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev0">http://cr.openjdk.java.net/~minqi/7164841/webrev0</a>2<br>
<br>
Thanks<br>
Yumin<br>
<br>
<div class="moz-cite-prefix">On 8/21/2013 3:43 PM, Yumin Qi
wrote:<br>
</div>
<blockquote cite="mid:521542A3.3020906@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Hi, all<br>
<br>
This is second version after feedback from first round.<br>
The changes are:<br>
<br>
1) file name will be based on gc log file name
(-Xloggc:filename), pid (process id) and time when the first
rotation file created:<br>
<filename>-pid<pid>-YYYY-MM-DD_HH-MM-SS<br>
2) If rotate in same file, file name is as in 1), if
rotate in multiple files, .<i> will append to above
file name.<br>
3) every time file rotated, file name and time when file
created will be logged to head/tail, this is same as first
version.<br>
4) current file has name
<filename>-pid<pid>-YYYY-MM-DD_HH-MM-SS.<i>.current<br>
This is similar to first version.<br>
By adapting such name format we will never loss logs
in case apps stops and restart, the log files will not be
overwritten since time stamp in file names.<br>
5) If open file failed, turn off gc log rotation. <br>
If due to some reason, file operation failed (such
as permission changed etc), with log file opened, logging
still works, but at <br>
saving and renaming, the file operation will fail,
this will lead not all files saved.<br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev01">http://cr.openjdk.java.net/~minqi/7164841/webrev01</a><br>
<br>
Tested with jtreg, JPRT.<br>
<br>
Thanks<br>
Yumin<br>
<br>
<div class="moz-cite-prefix">On 8/15/2013 8:35 AM, Yumin Qi
wrote:<br>
</div>
<blockquote cite="mid:520CF52B.6050000@oracle.com"
type="cite">
<meta http-equiv="content-type" content="text/html;
charset=UTF-8">
Hi, <br>
<br>
Can I have your review for this small changes?<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eminqi/7164841/webrev00/">http://cr.openjdk.java.net/~minqi/7164841/webrev00/</a><br>
<br>
This is for a enhancement to add head/tail message to
the logging files to assist reading GC output.<br>
1. modified prompt message if invalid arguments used
for log rotating;<br>
2. add time and file name message to log file
head/tail.<br>
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<br>
On Windows, there is no F_OK (existing test)
definition, F_OK is defined as "0" and for _access of
VC++, it just describes:<br>
<br>
<table style="border-collapse: collapse; padding: 0px;
width: 1306px; border: 1px solid rgb(187, 187, 187);
color: rgb(0, 0, 0); font-family: 'Segoe UI', 'Lucida
Grande', Verdana, Arial, Helvetica, sans-serif;
font-size: 13px; font-style: normal; font-variant:
normal; font-weight: normal; letter-spacing: normal;
line-height: 17px; orphans: auto; text-align: start;
text-indent: 0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px;">
<tbody>
<tr>
<th style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; background-color:
rgb(237, 237, 237); color: rgb(112, 112, 112);
text-align: left;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;"><span class="parameter"
style="font-style: italic;">mode</span><span
class="Apple-converted-space"> </span>value</p>
</th>
<th style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; background-color:
rgb(237, 237, 237); color: rgb(112, 112, 112);
text-align: left;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">Checks file for</p>
</th>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">00</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">Existence only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">02</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">Write-only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">04</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">Read-only</p>
</td>
</tr>
<tr>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">06</p>
</td>
<td style="border: 1px solid rgb(187, 187, 187);
margin: 10px; padding: 10px 8px; color: rgb(42,
42, 42); vertical-align: top;">
<p style="color: rgb(42, 42, 42); margin-top: 0px;
margin-bottom: 0px; padding-bottom: 0px;
line-height: 18px;">Read and write</p>
</td>
</tr>
</tbody>
</table>
<br>
<a moz-do-not-send="true"
href="http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx">http://msdn.microsoft.com/en-us/library/1w06ktdy.aspx</a><br>
The definition are consistent with unistd.h. <br>
<br>
Test: JPRT and jtreg.<br>
<br>
Thanks<br>
Yumin<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>