RFR: 8146879: Add option for handling existing log files in UL

[Marcus Larsson]

Hi,

On 03/11/2016 04:50 PM, Robbin Ehn wrote:
> Hi Marcus,
>
> Two small things.
>
> src/share/vm/runtime/os.hpp:
> +  static int compare_file_modified_times(const char* file1, const 
> char* file2);
> Is this so generic that we should have it os.[h,c]pp?
> E.g. if we want to compare ctime, etc.. ?

I didn't want to add a bunch of unused functionality for this so I kept 
it simple. If we need ctime or atime at a later point I think we should 
just refactor this or add it separately.

>
> 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).
>
> src/share/vm/logging/logFileOutput.hpp:
> + static const size_t DefaultRotationFileSize = 2097152; // 20MiB
> Missing a 0 here, I prefer to write this as 20*1024*1024

Great catch! I'll change this to use the M constant, making it 20*M.

>
>>
>> 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.
>
> I also functional tested this and it works as intended.
> There is an issue regarding removing the current log file.
> Discussed in side-channel it will be handled outside this CS since it 
> not directly related.
>
> Otherwise looks good!

Thanks for looking at it!
Marcus

>
> /Robbin
>
>>
>> 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
>>>>
>>>
>>