<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi, Kirk<br>
<br>
I just ignore all chars outside the [0-9][A-Z]a-z].-_, if p or t
doesn't follow %, whatever after % will be wrong and be rejected as
filename. That is no single % or %% allowance in the name.<br>
<br>
Thanks<br>
Yumin<br>
<br>
<br>
<div class="moz-cite-prefix">On 9/4/2013 10:10 AM, Kirk Pepperdine
wrote:<br>
</div>
<blockquote
cite="mid:70CF4B6B-0690-468C-B2C7-EB832B8AAEAB@kodewerk.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
Hi Yumin,
<div><br>
</div>
<div>Wow, this looks like a great incremental improvement. I agree
that it's wise to constrain the special characters for the file
name.</div>
<div><br>
</div>
<div>Can you not just say if %is followed by [plt] then the
substitution happens and if not the escaped % (\%) would be
inferred? Or, is that too weird or unexpected.</div>
<div><br>
</div>
<div>Regards,</div>
<div>Kirk</div>
<div><br>
<div><br>
</div>
<div>
<div>
<blockquote type="cite">
<div text="#000000" bgcolor="#FFFFFF"><br>
Is this OK?<br>
<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>
<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=ISO-8859-1"
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=ISO-8859-1"
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=ISO-8859-1">
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); font-size: 13px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: 17px; text-align: start;
text-indent: 0px; text-transform: none;
white-space: normal; 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;">
<div 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</div>
</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;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">Checks file for</div>
</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;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">00</div>
</td>
<td style="border: 1px solid rgb(187, 187,
187); margin: 10px; padding: 10px 8px;
color: rgb(42, 42, 42); vertical-align:
top;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">Existence only</div>
</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;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">02</div>
</td>
<td style="border: 1px solid rgb(187, 187,
187); margin: 10px; padding: 10px 8px;
color: rgb(42, 42, 42); vertical-align:
top;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">Write-only</div>
</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;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">04</div>
</td>
<td style="border: 1px solid rgb(187, 187,
187); margin: 10px; padding: 10px 8px;
color: rgb(42, 42, 42); vertical-align:
top;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">Read-only</div>
</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;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">06</div>
</td>
<td style="border: 1px solid rgb(187, 187,
187); margin: 10px; padding: 10px 8px;
color: rgb(42, 42, 42); vertical-align:
top;">
<div style="color: rgb(42, 42, 42);
margin-top: 0px; margin-bottom: 0px;
padding-bottom: 0px; line-height:
18px; ">Read and write</div>
</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>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>