Thanks for reviewing, Robbin!

Marcus

On 03/22/2016 03:24 PM, Robbin Ehn wrote:
Hi Marcus,

On 03/18/2016 02:04 PM, Marcus Larsson wrote:
Updated patch after feedback.

New webrev:
http://cr.openjdk.java.net/~mlarsson/8146879/webrev.04/

Incremental:
http://cr.openjdk.java.net/~mlarsson/8146879/webrev.03-04/

Tested with internal VM tests through RBT.

Changes:
* Rotation filecount is now limited to 1000 files.
* Removed loop in os::compare_file_modified_times.
* Added a check before rotating/truncating an existing log file, and
will only do so if it's a regular file.
* Added test case to check that logging to a directory gives the
intended error message.
* Fixed test help method to handle arbitrary length log lines.

I tested this and it works fine.

Code looks good to me.

Thanks for fixing this!

/Robbin


Thanks,
Marcus

On 03/11/2016 03:21 PM, Marcus Larsson wrote:
Third time's the charm.

Webrev:
http://cr.openjdk.java.net/~mlarsson/8146879/webrev.03/

This patch makes log file rotation the default. Default thresholds are
5 rotated files with a target size of 20MiB. Truncating behavior can
be achieved by setting filecount to 0 (-Xlog::myfile.log::filecount=0).

If a log file already exists during log file initialization it will be
rotated. If any of the target file names (file.0 to file.4 in the
default case) are available, that filename will be used for the
existing log. If all names are taken the VM will attempt to overwrite
the oldest file.

This should prevent unlimited log file creations and avoid accidental
loss of log files from previous runs. The default thresholds (5 files,
20MiB each) is just a suggestion. If you think it should be
higher/lower let me know.

Tested with included internal VM tests through RBT.

Thanks,
Marcus

On 2016-03-01 15:05, Marcus Larsson wrote:
Hi,

After some offline discussions I'm withdrawing this patch. I will
instead investigate if I can achieve similar behavior using log
rotation as the default.

Thanks,
Marcus

On 03/01/2016 12:11 PM, Marcus Larsson wrote:
Hi again,

Taking a different approach to this.

New webrev:
http://cr.openjdk.java.net/~mlarsson/8146879/webrev.01/

Existing files will now by default be renamed/archived with a .X
suffix where X is the lowest number such that the resulting file
name is available (jvm.log becomes jvm.log.0). A mode option for
controlling this behavior has been added as well. It can be set to
archive, append, or truncate (i.e. -Xlog::jvm.log::mode=truncate).

Tested with included jtreg test through JPRT.

Thanks,
Marcus

On 01/14/2016 04:00 PM, Marcus Larsson wrote:
Hi,

Please review the following patch to make sure UL truncates
existing log files before writing to them. Since files are opened
in append mode, truncation isn't done automatically, so instead the
patch adds an attempt to remove the log file before opening it.

Webrev:
http://cr.openjdk.java.net/~mlarsson/8146879/webrev.00/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8146879

Testing:
Included test through JPRT

Thanks,
Marcus





Reply via email to