Hi,
On 2015-10-27 01:03, Yasumasa Suenaga wrote:
Hi Marcus,
Thank you for replying.
I filed this feature to JBS and change subject of this email.
Could you be a sponsor and review it?
I'll sponsor it.
http://cr.openjdk.java.net/~ysuenaga/JDK-8140556/webrev.00/
logConfiguration.cpp/hpp:
* I don't think we should rely on the archive_name or the output's name
to decide whether or not an output should be rotated. It would be better
to introduce an is_rotated() test function in LogOutput that could be
used here.
* rotate_all_logfile() should be renamed to rotate_all_outputs().
Currently there are only LogFileOutputs (other than stdout/stderr), but
in the future there might be other types of outputs so I prefer having a
more general name.
Also, please use static_cast<LogFileOutput*> instead of the C-style
cast. (Additional logic will be required here once more types of log
outputs are added, but I don't think we need to worry about this right now.)
* Don't print an error if an output is not rotatable, since it's not an
error to have some log outputs rotated while others are not. If you
really want some traceability here I suggest adding log messages on
trace level, tagged with 'logging'.
logDiagnosticCommand.cpp/hpp:
* I think the description could be improved to something like "Lists
current log configuration, enables/disables/configures a log output, or
disables/rotates all logs."
* The rotate option description should clarify that all logs will be
rotated ("current log" is too ambiguous).
logFileOutput.cpp/hpp:
* Moving the MutexLocker like this introduces a race condition where a
log might be rotated multiple times by different threads, instead of
just once.
Instead of making the rotate() function public and moving the
mutexlocker, I suggest adding something like a public force_rotation()
function that grabs the lock and calls the private rotate().
* Given the addition of is_rotated() in LogOutput, then
get_archive_name() should be removed.
Thanks,
Marcus
Thanks,
Yasumasa
On 2015/10/26 18:56, Marcus Larsson wrote:
Hi,
Sorry for my late reply.
I think being able to force rotation via jcmd seems like a good
feature. Files are currently opened in append mode so it should
already be possible to use external log rotation tools by copying and
truncating the files. Still I think it would be nice to provide the
jcmd for rotation as well.
I can see some small issues with the implementation, but we can deal
with that during the review.
Thanks,
Marcus
On 2015-10-26 00:26, Yasumasa Suenaga wrote:
Hi all,
Have you ever seen my change?
I (and my customers) need log rotation function via external tool.
I want to merge it by Feature Complete.
Thanks,
Yasumasa
2015/10/16 22:55 "Yasumasa Suenaga" <yasue...@gmail.com>:
Hi all,
I contributed JDK-7090324: gclog rotation via external tool to be
synchronized with
logrotated tool on Linux.
I think JEP 158 is in progress.
However, this JEP does not contain log rotation via external tool
in the
spec.
I want to rotate logs via jcmd in this JEP.
I've updated a patch for it:
http://cr.openjdk.java.net/~ysuenaga/jvmlogging-logrotate/
This patch provides new option "rotate" in VM.log command.
If this change can be accepted, I will file it to JBS and send RFR.
Thanks,
Yasumasa
