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

[Marcus Larsson]

Hi Gerard,

On 03/22/2016 07:15 PM, Gerard Ziemski wrote:
> hi Marcus,
>
> Thank you for incorporating the feedback.
>
> The only remaining comment I have is about the “number_of_lines_with_substring_in_file()” function. As written, it resets the entire algorithm to the beginning of the file when it can not fit the current line, when I think the intention should be only to rewind only to the beginning of that line.
>
> May I suggest the following implementation instead:
>
> static size_t number_of_lines_with_substring_in_file(const char* filename,
>                                                           const char* substr) {
>   ResourceMark rm;
>   size_t ret = 0;
>   FILE* fp = fopen(filename, "r");
>   assert(fp, "error opening file %s: %s", filename, strerror(errno));
>
>   int buflen = 512;
>   char* buf = NEW_RESOURCE_ARRAY(char, buflen);
>   long int pos = 0;
>
>   while (fgets(buf, buflen, fp) != NULL) {
>     if (buf[strlen(buf) - 1] != '\n') {
>       // rewind to the beginning of the line
>       fseek(fp, pos, SEEK_SET);
>       // double the buffer size and try again
>       buf = REALLOC_RESOURCE_ARRAY(char, buf, buflen, 2*buflen);
>       buflen *= 2;
>     } else {
>       // get current file position
>       pos = ftell(fp);
>
>       if (strstr(buf, substr) != NULL) {
>         ret++;
>       }
>     }
>   }
>
>   fclose(fp);
>   return ret;
> }

Alright, fixed.

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

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

Also fixed some failing tests caused by this patch in the previous round 
of changes.

Thanks,
Marcus

>
> cheers
>
>
>> On Mar 18, 2016, at 8:04 AM, Marcus Larsson <marcus.larsson at oracle.com> 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.
>>
>> 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