Bugzilla – Full Text Bug Listing
| Summary: | Log size limits and rotating logs | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Ilja Preuss <preuss> | ||||
| Component: | Runtime | Assignee: | Pascal Rapicault <pascal> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | douglas.pollock, julianc, loats, mlists, wassim.melhem | ||||
| Version: | 3.1 | Keywords: | helpwanted | ||||
| Target Milestone: | 3.1 M7 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Ilja Preuss
The size check is interesting. A complementary approach might be to have a rotating log (I think there is another bug about that but I can't find it). Marking as "helpwanted" since this is a pretty standalone item that could be investigated by the community. See bug 14223 comment 8, and bug 22765. Personally, I like the idea of warning the user when the log reaches a certain limit and offer to delete it/part of it. On embedded RCP apps it will probably make sense to limit the size of the log and never allow it to grow past a certain size. Some scenarios (maybe most) would not want to warn the user, but rather just keep the file a reasonable size without user intervention. A rotating log would be OK, too; I don't really need that warning... I think the warning only makes sense for a certain set of users (most likely IDE developers), but that would not work in RCP use cases. Personnaly I would go for the rotating log without warning, where the max size can be defined by a property. Allowing the .log file to grow without bound is actually dangerous. This affects the stability of Eclipse, and other applications on the system. I would argue that this is a bug (not an enhancement), and that a default limit should be put in place. Phil don't you have some code for this kind of problem? This problem is on the eRCP to-do list. I have not addressed the problem yet. The eRCP team will start looking into this. Do you think you'll have a chance to solve that before we ship 3.1? I discussed this with Phil. We should be able to use a solution that does not impact the EclipseLog code too much and is acceptable for Eclipse 3.1 M7. Here is the strategy we came up with. - use a property to specify a max log size (eclipse.log.max ?) - before opening the log file, check the size of the file - if the file size is greater that the eclipse.log.max then move the log file to <logfilename>.bak. Since this is java, we will have to copy the file and delete the original. - if eclipse.log.max=0 then there is no max and we never rotate to a bak file - only keep one bak file (maybe this could be configurable) - should default the value to something that is generally acceptable for the desktop SDK. (i.e. 5 megs, 10 megs ?) This fix will be isolated to the EclipseLog class. "Since this is java, we will have to copy the file and delete the original." Tom, could you clarify on why File.rename() is not appropriate? opps, you are right Rafael. I don't know what I was thinking ... File#renameTo(File) should work fine. Ideally, this should also be able to handle the case where the log file is
filling up while Eclipse is running. Perhaps in the method write(String), there
could be a line:
if (message != null) {
File file = getFile();
if ((file != null) && (file.length() > SOME_LIMIT)) {
rotateLog();
}
writer.write(message);
...
}
Douglas, you are correct. That is intention of this solution. The log file is opened and closed for each log entry. We would do the size check each time before opening the log file to write to it. If a particular log entry is large then a log file may overstep the bounds of the max, but on the next log entry we would move the log to the .bak file. If the log file could be moved to the backup with each entry... - do we re-write the SESSION header in the new log? - do/should we somehow note that the entries are continued from the backup re comment 16: - We should write the SESSION header to the new log - there should probably be some indication that this SESSION is a continuation from the backup log. Any suggestions on how to do that? An entry saying "this is a continuation of log file <backup log file location>"? Wassim, any thoughts since you are the log-parsing guy? Long log files have been a concern for PDE for some time now. What we have done since 3.0 is that we arbitrarily set s magic number to be 1M If the log file is > 1 M, we only read the last 1M. This works well for us. Prior to doing that, the log view was bringing the workbench to its knees when trying to read 250M files. I was thinking more along the lines of how the Error Log view would behave if the workspace log was spread across multiple files. Should the user know/care about this? In most cases when you get lots of exception its the first one that is important because it will cause the trickle-down effect on other problems. I'm just worried about the max size being set too low (by the user) and then us receiving log files with only the last half of the stacks. Another thing we used to have was a rolling log file. So you said you wanted 5 files of max length 1meg each and it logged that way and spill-over went to the next one. (similar to the regular .bak suggestion but you'd just have 5 .bak files) Or maybe I'm just thinking too much for a Friday... Only when we first read the log upon startup is when PDE actually deals with the .log file directly. That is the time when we read as much/little as we want. After that, we register a log listener and log events to the view on our own. So if you want to break up the log into multiple files or whatever, PDE could read the latest one (based on timestamp?) I suppose as a starting point and then it's business as usual actually, as long as the most recent log is named '.log', PDE will have to do nothing special, which is great. If people want to see individual older logs, they could use the import function in the log view. This is Julian Chen, and I'm working on the log file size limitation function. According to all comments up to now, the following is my summary and proposal. Please let me know if you think anything is not ok. There will be two new defined system properties. "eclipse.log.size.max" : The max size of log file. Data Type: Integer Default value when no assigned: 1000(KB) if (value==0) then it means no size limitation if (value<10) then set value as 10(KB) (I think too small log size is not appropriate.) "eclipse.log.backup.max" : The max amount of backup log file to keep. Data type: Integer Default value when no assigned: 1 Value's min is '1' (at least one backup) When the log() method is called to write log to file, it'll check current log file size. If the size oversteps the max value, then the log file will be renamed to <log_file_name>.<index>."bak" For example, The first backup log file name will be <log_file_name>.0.bak The second backup log file name will be <log_file_name>.1.bak The third backup log file name will be <log_file_name>.2.bak if the index is greater than the max ammount of backup logs, then it is reset to '0' and the oldest one will be overwritten. New log file will be created and new SESSION header and the following String will be written in the file header. "This is a continuation of log file "+ <previous file name> (Will it have any impact on log-parsing ?) The log view only cares about two things: 1. the most recent log must remain named '.log'. 2. The import function in the log view expects a .log extension, so for the backups to be able to be imported, they have to be named *.bak.log Following is an example of a session header. You are thinking of putting that message along with the other info (below the header?), correct? !SESSION 2005-04-19 15:31:16.357 ----------------------------------------------- eclipse.buildId=I20050414-1107 java.version=1.5.0 java.vendor=Sun Microsystems Inc. BootLoader constants: OS=win32, ARCH=x86, WS=win32, NL=en_CA Framework arguments: -keyring c:\Documents and Settings\rchaves\.keyring -showlocation Command-line arguments: -os win32 -ws win32 -arch x86 -keyring c:\Documents and Settings\rchaves\.keyring -consolelog -console -showlocation -debug Do we require multiple log file backups for the workbench logs? Would keeping at most one single backup be sufficient as more data can be kept by increasing the max size? I think for the configuration/[timestamp].log files, we should keep an adjustable number of past log files as these will always be one log file per session. I'm going to change the backup log filename as The first backup log file name will be <log_file_basename>.bak_0.log The second backup log file name will be <log_file_basename>.bak_1.log The third backup log file name will be <log_file_basename>.bak_2.log And, the new log header will look like !SESSION 2005-04-28 23:12:46.904 ---------------------------------------------- - eclipse.buildId=unknown java.version=1.4.2_01 java.vendor=Sun Microsystems Inc. BootLoader constants: OS=win32, ARCH=x86, WS=win32, NL=zh_TW Framework arguments: -application org.eclipse.ercp.example.ercpHello.ercpHello Command-line arguments: -application org.eclipse.ercp.example.ercpHello.ercpHello -console This is a continuation of log file e:\eRCP\image\configuration\1114701137982.bak_0.log !ENTRY org.eclipse.osgi 2005-04-28 23:12:46.904 !MESSAGE ... Then it should be easier for log view to read the data. Created attachment 20595 [details]
Patch to EclipseLog.java
This patch provides the configurable limitation of log file size.
I started the review of the patch. The patch has been released in HEAD. I changed the following things: - the value in the property are expected to be strings - the code has been formated I kept the return value of checkLogFileSize() even though the value is never used. What was the intent here. Instead of keeping the same behavior than in 3.0 (one big unique log file), I deliberatly chose to enable the support and create a new file whenever the current file is bigger than 1M. If you think this can be a problem let me know. I updated to code to protect the callers (using doPriveledge) when security is enabled. Otherwise SecurityExceptions could be thrown while accessing the system properties. Updating title to accurately reflect what was done. (In reply to comment #31) > The patch has been released in HEAD. > I changed the following things: > - the value in the property are expected to be strings > - the code has been formated > I kept the return value of checkLogFileSize() even though the value is never used. > What was the intent here. It's not used now, just a info to tell caller if it failed to backup log file. > Instead of keeping the same behavior than in 3.0 (one big unique log file), I > deliberatly chose to enable the support and create a new file whenever the > current file is bigger than 1M. Not sure if I understand it. This patch enables this support by default, right ? It is just that I hesitated to turn on this support by default, but I finally did it. I'd like to see these options in the preferences, maybe along with the PDE options for the log (only show current session, sort by .. etc.). Any plan on doing so? Thanks :) HH |