Log4net
  1. Log4net
  2. LOG4NET-27

Rolling files on date/time boundaries doesn't support a maximum number of backup files.

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.2.11
    • Fix Version/s: 1.2 Maintenance Release
    • Component/s: Appenders
    • Labels:
      None

      Description

      A maximum of backup files exist when rolling files on file size, but not for rolling on date/time.

      This can be implemented with the same config key : MaxSizeRollBackups

      1. RollingFileAppender.zip
        67 kB
        Hans Meier
      2. RollingFileAppender.patch
        1 kB
        Florian Ramillien
      3. rollingfileappender.diff
        3 kB
        Jochen Kühner
      4. RollingFileAppender.cs.patch
        5 kB
        Florian Ramillien
      5. RollingFileAppender.cs
        58 kB
        Joshua Masek
      6. RollingFileAppender.cs
        50 kB
        jcflores
      7. RollingFileAppender.cs
        51 kB
        Jochen Kühner
      8. LOG4NET-27.patch
        8 kB
        Petr Felzmann
      9. appender_diff
        3 kB
        Jochen Kühner

        Issue Links

          Activity

          Hide
          Mark Ward added a comment -

          the log4net release 1.2.12 appears to break this posted solution. I have a derived version from the RollingFileAppender.zip that worked with the previous release of log4net but when ran against 1.2.12 it fails to pass its tests.

          Show
          Mark Ward added a comment - the log4net release 1.2.12 appears to break this posted solution. I have a derived version from the RollingFileAppender.zip that worked with the previous release of log4net but when ran against 1.2.12 it fails to pass its tests.
          Hide
          Dominik Psenner added a comment -

          Hi Hans. Thanks for your help! Unfortunately the patch you provided is too blown up to be reviewed easily. I reviewed it on the devel mailing list as far as I was able to. Would you please check the mailing list messages and let us know what you think?

          Show
          Dominik Psenner added a comment - Hi Hans. Thanks for your help! Unfortunately the patch you provided is too blown up to be reviewed easily. I reviewed it on the devel mailing list as far as I was able to. Would you please check the mailing list messages and let us know what you think?
          Hide
          Hans Meier added a comment -

          In parallel to the last activities here I started my own implementation of the "max number of date roll backups" feature on 14. January based on trunk rev 1431235. After this day I didn't check for activities here because it has been so quiet for so long so I missed that something was going on in parallel.

          So here it is. As far as I can see it does exactly what is intended.

          • limit the number of periods logs are kept
          • roll of old files upon application start works as expected
          • composite mode works, size limits apply for each period separately
          • count directions forward and backward work as expected
          • preserve log file extension works as expected
          • using static log file works as expected
          • adding a footer after end of the respective period doesn't affect date indexing (option layout->footer). last write time is only used for static log files any more because using last write time confused the roll mechanism when a footer is configured. with footer the last write time always is out-of-period for the last logfile of the period.

          I tested it with several combinations of settings. I also added some tests. Unfortunately there were not so many tests regarding roll over date so I just added some tests for some details I was particularly interested in.

          To ease reviewing and committing the zip attached contains 4 steps:

          step1:
          only trivial changes:

          • tabs and spaces, formatting etc.

          step2:
          a bugfix.

          • in RollOverIfDateBoundaryCrossing preserveLogFileExtension wasn't respected.

          step3:
          modifications without affecting existing code.

          • some trivial changes in visibility, regions etc.
          • additional functions, properties, members that aren't yet used from existing code.
          • NextCheckDate -> GetRollDateTimeRelative:
            Function works identically if called with relativePeriod==1. Existing code does this.
            Additionally any count of positive or negative period jumps can be calculated.
          • Fully isolated getting last write time in a function GetLastWriteTime
          • Split CombinePath into a static version and a member function to call it. Needed
            to allow implementing and testing one of the new functions as static function.
            That new, static function uses CombinePath so i needed a static version.

          step4:
          use the new functionality

          I hope that helps ...

          Show
          Hans Meier added a comment - In parallel to the last activities here I started my own implementation of the "max number of date roll backups" feature on 14. January based on trunk rev 1431235. After this day I didn't check for activities here because it has been so quiet for so long so I missed that something was going on in parallel. So here it is. As far as I can see it does exactly what is intended. limit the number of periods logs are kept roll of old files upon application start works as expected composite mode works, size limits apply for each period separately count directions forward and backward work as expected preserve log file extension works as expected using static log file works as expected adding a footer after end of the respective period doesn't affect date indexing (option layout->footer). last write time is only used for static log files any more because using last write time confused the roll mechanism when a footer is configured. with footer the last write time always is out-of-period for the last logfile of the period. I tested it with several combinations of settings. I also added some tests. Unfortunately there were not so many tests regarding roll over date so I just added some tests for some details I was particularly interested in. To ease reviewing and committing the zip attached contains 4 steps: step1: only trivial changes: tabs and spaces, formatting etc. step2: a bugfix. in RollOverIfDateBoundaryCrossing preserveLogFileExtension wasn't respected. step3: modifications without affecting existing code. some trivial changes in visibility, regions etc. additional functions, properties, members that aren't yet used from existing code. NextCheckDate -> GetRollDateTimeRelative: Function works identically if called with relativePeriod==1. Existing code does this. Additionally any count of positive or negative period jumps can be calculated. Fully isolated getting last write time in a function GetLastWriteTime Split CombinePath into a static version and a member function to call it. Needed to allow implementing and testing one of the new functions as static function. That new, static function uses CombinePath so i needed a static version. step4: use the new functionality I hope that helps ...
          Hide
          Dominik Psenner added a comment -

          I appreciate your effort Jochen, but I'm not going to accept patches that destroy data by design.

          Is there a special reason why you did not integrate your code into the method "AdjustFileBeforeAppend()"?

          Show
          Dominik Psenner added a comment - I appreciate your effort Jochen, but I'm not going to accept patches that destroy data by design. Is there a special reason why you did not integrate your code into the method "AdjustFileBeforeAppend()"?
          Hide
          Jochen Kühner added a comment -

          1.) I don't see how i could fix this. Maybe we need to warn the usern in the documentation that the use of this Property could be dangerous. But I find the solution to store the old log file names not good!

          2.) The check is on every Append call. After that it checks if there are files wich should be deleted

          4-5 should be fixed in my new patch

          Show
          Jochen Kühner added a comment - 1.) I don't see how i could fix this. Maybe we need to warn the usern in the documentation that the use of this Property could be dangerous. But I find the solution to store the old log file names not good! 2.) The check is on every Append call. After that it checks if there are files wich should be deleted 4-5 should be fixed in my new patch
          Hide
          Dominik Psenner added a comment -

          Thanks for your help! If I read your patch correctly, you retrieve a list of filenames from the directory where m_baseFileName is stored at and delete the oldest ones if the number of files is bigger than m_maxSizeRollBackups based on the LastWriteTime property on the file.

          I'm not accepting the patch as-is, but it steers into the right direction. The reasons why I'm not accepting it:

          [1] it eventually deletes unrelated files if m_BaseFileName is configured badly (i.e. a users home folder)
          [2] what does the m_trimFilesDaily property do? According to its name it deletes the files daily, but where is the "daily check"? It would seem to me that part of the code does not change the behaviour, does it?

          and the refactorable minor issues are:

          [3] missing documentation
          [4] duplicated code
          [5] the indentation of the patch are spaces where it should be tabs instead

          Would you please look into fixing [1] and [2]?

          Show
          Dominik Psenner added a comment - Thanks for your help! If I read your patch correctly, you retrieve a list of filenames from the directory where m_baseFileName is stored at and delete the oldest ones if the number of files is bigger than m_maxSizeRollBackups based on the LastWriteTime property on the file. I'm not accepting the patch as-is, but it steers into the right direction. The reasons why I'm not accepting it: [1] it eventually deletes unrelated files if m_BaseFileName is configured badly (i.e. a users home folder) [2] what does the m_trimFilesDaily property do? According to its name it deletes the files daily, but where is the "daily check"? It would seem to me that part of the code does not change the behaviour, does it? and the refactorable minor issues are: [3] missing documentation [4] duplicated code [5] the indentation of the patch are spaces where it should be tabs instead Would you please look into fixing [1] and [2] ?
          Hide
          Jochen Kühner added a comment -

          I've attached the Trimm files Daily patch as a Diff for current SVN!

          Show
          Jochen Kühner added a comment - I've attached the Trimm files Daily patch as a Diff for current SVN!
          Hide
          Jochen Kühner added a comment -

          Patch for Trimming Files Daily...

          Show
          Jochen Kühner added a comment - Patch for Trimming Files Daily...
          Hide
          Dominik Psenner added a comment -

          So, the general consensus is that rolling should be rewritten to use a persistent storage for the filenames to be removed. However I don't see this feature to fit well into the current implementation of the RollingFileAppender. But the rewrite of the RollingFileAppender should support this feature.

          Therefure I created the issue LOG4NET-367 and marked this one and other RollingFileApppender related issues to be superceded by the new issue.

          Show
          Dominik Psenner added a comment - So, the general consensus is that rolling should be rewritten to use a persistent storage for the filenames to be removed. However I don't see this feature to fit well into the current implementation of the RollingFileAppender. But the rewrite of the RollingFileAppender should support this feature. Therefure I created the issue LOG4NET-367 and marked this one and other RollingFileApppender related issues to be superceded by the new issue.
          Hide
          Spamme added a comment -

          You can try to guess the old file names but you can't never be sure, perhaps the user has changed the string format, in this case how do you find them? You can try to solve this problem in many ways but at the end there isn't a sure way to do it. For example:

          • You could use the file name without the date format to find all the log files and use the file date time to determine which log file is older
            --> The user could have two loggers in the same folder one with the file name "logger.log" and the other with the name "2nd logger.log", so looking for "logger.log" in the file name would find both logger files.
            --> The user could have opened the file in a editor and just saved it and the log file has now the actual date and time, so the date time of the file can't be used
          • You could add an header inside each log file to recognize them, for example: "LoggerName" + UTC Date, and scan all the files for the header.
            --> The user could have renamed a log file to prevent to be deleted, but in this case would be deleted too
            --> You have to open all the files to read the header, which is not really an efficient solution

          So use the KISS and go for the standard case, which will cover 99% of the cases:

          • The user configure the logger for deleting the old log files.
            --> From this point the logger remembers the generate file and delete them when their time has come
          Show
          Spamme added a comment - You can try to guess the old file names but you can't never be sure, perhaps the user has changed the string format, in this case how do you find them? You can try to solve this problem in many ways but at the end there isn't a sure way to do it. For example: You could use the file name without the date format to find all the log files and use the file date time to determine which log file is older --> The user could have two loggers in the same folder one with the file name "logger.log" and the other with the name "2nd logger.log", so looking for "logger.log" in the file name would find both logger files. --> The user could have opened the file in a editor and just saved it and the log file has now the actual date and time, so the date time of the file can't be used You could add an header inside each log file to recognize them, for example: "LoggerName" + UTC Date, and scan all the files for the header. --> The user could have renamed a log file to prevent to be deleted, but in this case would be deleted too --> You have to open all the files to read the header, which is not really an efficient solution So use the KISS and go for the standard case, which will cover 99% of the cases: The user configure the logger for deleting the old log files. --> From this point the logger remembers the generate file and delete them when their time has come
          Hide
          Jochen Kühner added a comment -

          Hopefully tomorrow I've time to update my patch (wich is not my invention, i only updated a old Version here) to work with the newest SVN.

          Show
          Jochen Kühner added a comment - Hopefully tomorrow I've time to update my patch (wich is not my invention, i only updated a old Version here) to work with the newest SVN.
          Hide
          Dominik Psenner added a comment - - edited

          If a persistent storage of the file history is not needed, why keep it anyway? I think that the implementation of the rolling could always use a persistent history somewhere (registry|file|..?) allowing the implementation of fancier rolling methods that even combine date, size and arbitrary (random) filenames. Currently the rolling is quite limited from that point of view. Since a rewrite of the rollingfileappender is in discussion since long time those ideas could well make it into there. The discussion took place at http://apache-logging.6191.n7.nabble.com/RFA-NG-review-td24505.html and some helping hands / comments / reviews are still welcome. Somehow I (and probably also Stefan) lost track on the RFA-NG topic since I didn't need it anymore and have not enough spare time to work on it even if it would resolve a lot of log4net issues.

          Show
          Dominik Psenner added a comment - - edited If a persistent storage of the file history is not needed, why keep it anyway? I think that the implementation of the rolling could always use a persistent history somewhere (registry|file|..?) allowing the implementation of fancier rolling methods that even combine date, size and arbitrary (random) filenames. Currently the rolling is quite limited from that point of view. Since a rewrite of the rollingfileappender is in discussion since long time those ideas could well make it into there. The discussion took place at http://apache-logging.6191.n7.nabble.com/RFA-NG-review-td24505.html and some helping hands / comments / reviews are still welcome. Somehow I (and probably also Stefan) lost track on the RFA-NG topic since I didn't need it anymore and have not enough spare time to work on it even if it would resolve a lot of log4net issues.
          Hide
          Joshua Masek added a comment -

          I'm glad there is still chatter on this issue, which shows there is other interest out there for a solution.

          I uploaded a solution a few years back. Not saying that it was awesome code, but I think it works. It made me realize how this feature wasn't as simple as it sounded.

          Some comments geared towards recent discussion:

          • it would need to roll by date AND by maxSizeRollBackups
          • it would need to delete files where the date could be much older than the time period's current range. So if we are keeping the last 20 days of log files where the log filename has the date in it, it should be able to delete a log file from last year (along with any files rolled over based on maxSizeRollBackups for that date), as the application might not have been run in awhile.
          • as for the ".history.txt" idea, it sounds good, at first anyways, as I haven't thought about it much. If this file were missing, it could use an algorithm to basically try to auto-detect the files that should be in this list and recreate it. (Like I imagine one could change my patch or write a new one to instead of detecting files to delete, rather detect files that should be in this .history.txt file.) Although not sure if it is a big deal that if the history file gets deleted, we just leave the logs there as forgotten.
          Show
          Joshua Masek added a comment - I'm glad there is still chatter on this issue, which shows there is other interest out there for a solution. I uploaded a solution a few years back. Not saying that it was awesome code, but I think it works. It made me realize how this feature wasn't as simple as it sounded. Some comments geared towards recent discussion: it would need to roll by date AND by maxSizeRollBackups it would need to delete files where the date could be much older than the time period's current range. So if we are keeping the last 20 days of log files where the log filename has the date in it, it should be able to delete a log file from last year (along with any files rolled over based on maxSizeRollBackups for that date), as the application might not have been run in awhile. as for the ".history.txt" idea, it sounds good, at first anyways, as I haven't thought about it much. If this file were missing, it could use an algorithm to basically try to auto-detect the files that should be in this list and recreate it. (Like I imagine one could change my patch or write a new one to instead of detecting files to delete, rather detect files that should be in this .history.txt file.) Although not sure if it is a big deal that if the history file gets deleted, we just leave the logs there as forgotten.
          Hide
          Spamme added a comment -

          Yes, it is expecting that you have exactly 1 log file per each possible changing, I know is not a perfect solution. The better approach would be to save all the file names in a another file, when you create a new file you add it to the list and you pop the old ones and delete them. You have only to find an unique name for the history file, for example LoggerName.history.txt. This is also an imperfect solution, because if somebody deletes the history file ... you can't delete the old files anymore. But at the end there isn't a perfect solution, you have always to chose what fits at best your needs. This solution is safe against any change in the configuration, if you want you could set the history file hidden, so it would be more difficult for a normal user to delete it, or you could also open it and lock it, so nobody can change or delete it as long as the application is running, but personally I don't like this method.

          protected void RollOverTime(bool fileIsOpen)
          {
          ...
          //Delete old files
          if (m_maxSizeRollBackups != 0)
          {
          //Opening the history file
          using (FileStream fs = new FileStream(Name + ".history.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite))
          {
          try
          {
          //Reading the content of the file and transform it to a list
          var list = new List<string>((new StreamReader(fs).ReadToEnd()).Split(new char[]

          { ';' }

          , StringSplitOptions.RemoveEmptyEntries));

          //Adding the new file name
          list.Add(dateFormat);

          //While the list is longer than the max length
          while (list.Count > m_maxSizeRollBackups)

          { //Getting the first file name in the list var oldFile = CombinePath(File, list[0]); //Deleting the first file name if it exists if (FileExists(oldFile)) DeleteFile(oldFile); //Removing the first file name list.RemoveAt(0); }

          //Repositioning the stream to the begin
          fs.Position = 0;
          //Write the new list to the file
          new StreamWriter(fs)

          { AutoFlush = true }

          .Write(string.Join(";", list));
          //Clearing the rest of the stream
          fs.SetLength(fs.Position);
          }
          //Closing the file
          finally

          { fs.Close(); }

          }
          }
          ....
          }

          Show
          Spamme added a comment - Yes, it is expecting that you have exactly 1 log file per each possible changing, I know is not a perfect solution. The better approach would be to save all the file names in a another file, when you create a new file you add it to the list and you pop the old ones and delete them. You have only to find an unique name for the history file, for example LoggerName.history.txt. This is also an imperfect solution, because if somebody deletes the history file ... you can't delete the old files anymore. But at the end there isn't a perfect solution, you have always to chose what fits at best your needs. This solution is safe against any change in the configuration, if you want you could set the history file hidden, so it would be more difficult for a normal user to delete it, or you could also open it and lock it, so nobody can change or delete it as long as the application is running, but personally I don't like this method. protected void RollOverTime(bool fileIsOpen) { ... //Delete old files if (m_maxSizeRollBackups != 0) { //Opening the history file using (FileStream fs = new FileStream(Name + ".history.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite)) { try { //Reading the content of the file and transform it to a list var list = new List<string>((new StreamReader(fs).ReadToEnd()).Split(new char[] { ';' } , StringSplitOptions.RemoveEmptyEntries)); //Adding the new file name list.Add(dateFormat); //While the list is longer than the max length while (list.Count > m_maxSizeRollBackups) { //Getting the first file name in the list var oldFile = CombinePath(File, list[0]); //Deleting the first file name if it exists if (FileExists(oldFile)) DeleteFile(oldFile); //Removing the first file name list.RemoveAt(0); } //Repositioning the stream to the begin fs.Position = 0; //Write the new list to the file new StreamWriter(fs) { AutoFlush = true } .Write(string.Join(";", list)); //Clearing the rest of the stream fs.SetLength(fs.Position); } //Closing the file finally { fs.Close(); } } } .... }
          Hide
          Dominik Psenner added a comment - - edited

          Hey Spamme.

          I'm glad to see some new name around here. Doesn't your code snippet remove also recent files? Example:

          let m_maxSizeRollBackups=30

          and assume the application rolled exactly 30 seconds ago. Then your snippet would would remove the file, even though there are only 2 logfiles, wouldn't it? I believe the rolling code should find existing files that have been rolled over and remove the oldest ones until their count is below m_maxSizeRollBackups.

          Cheers

          Show
          Dominik Psenner added a comment - - edited Hey Spamme. I'm glad to see some new name around here. Doesn't your code snippet remove also recent files? Example: let m_maxSizeRollBackups=30 and assume the application rolled exactly 30 seconds ago. Then your snippet would would remove the file, even though there are only 2 logfiles, wouldn't it? I believe the rolling code should find existing files that have been rolled over and remove the oldest ones until their count is below m_maxSizeRollBackups. Cheers
          Hide
          Spamme added a comment -

          Hello, I find really funny, that this issue has been open 8 years ago and there isn't a solution for it yet. So I don't want to do all the job, I can give you some code and perhaps you can improve and test it. (Why? Because I have only take a look to the RollingFileAppender.cs code and I didn't checked out the whole source)

          protected void RollOverTime(bool fileIsOpen)
          {
          ....
          //Delete old file
          if (m_maxSizeRollBackups != 0)
          {
          //Possible intervals for the date format changing: 1 sec, 1 min, 1 hours, 1 day, 1 week, 1 month, 1 year (this could be also a constant)
          //Note: using 31 days for the month and 366 days (leap year) for the year,
          //shouldn't be a problem because the function is called when the log file name has already changed
          var intervals = new double[]

          { 1, 60, 60 * 60, 60 * 60 * 24, 60 * 60 * 24 * 7, 60 * 60 * 24 * 31, 60 * 60 * 24 * 366 }

          ;

          foreach (var interval in intervals)
          {
          //Traveling back in time, weeeee.....
          var oldDate = m_now.AddSeconds(-interval);
          var oldDateFormat = oldDate.ToString(m_datePattern, System.Globalization.DateTimeFormatInfo.InvariantInfo);

          if (oldDateFormat != dateFormat)

          { //Bingo, date format changed, we have found the minimal interval for date format changing //Now calculate the the date format back m_maxSizeRollBackups times, time traveling again, weeeeee..... oldDate = m_now.AddSeconds(-interval * m_maxSizeRollBackups); oldDateFormat = oldDate.ToString(m_datePattern, System.Globalization.DateTimeFormatInfo.InvariantInfo); //Create the old file name and checking if exists, if exists delete it var oldFileName = CombinePath(File, oldDateFormat); if (FileExists(oldFileName)) DeleteFile(oldFileName); //we are done, break it break; }

          }
          }
          ...
          }

          Show
          Spamme added a comment - Hello, I find really funny, that this issue has been open 8 years ago and there isn't a solution for it yet. So I don't want to do all the job, I can give you some code and perhaps you can improve and test it. (Why? Because I have only take a look to the RollingFileAppender.cs code and I didn't checked out the whole source) protected void RollOverTime(bool fileIsOpen) { .... //Delete old file if (m_maxSizeRollBackups != 0) { //Possible intervals for the date format changing: 1 sec, 1 min, 1 hours, 1 day, 1 week, 1 month, 1 year (this could be also a constant) //Note: using 31 days for the month and 366 days (leap year) for the year, //shouldn't be a problem because the function is called when the log file name has already changed var intervals = new double[] { 1, 60, 60 * 60, 60 * 60 * 24, 60 * 60 * 24 * 7, 60 * 60 * 24 * 31, 60 * 60 * 24 * 366 } ; foreach (var interval in intervals) { //Traveling back in time, weeeee..... var oldDate = m_now.AddSeconds(-interval); var oldDateFormat = oldDate.ToString(m_datePattern, System.Globalization.DateTimeFormatInfo.InvariantInfo); if (oldDateFormat != dateFormat) { //Bingo, date format changed, we have found the minimal interval for date format changing //Now calculate the the date format back m_maxSizeRollBackups times, time traveling again, weeeeee..... oldDate = m_now.AddSeconds(-interval * m_maxSizeRollBackups); oldDateFormat = oldDate.ToString(m_datePattern, System.Globalization.DateTimeFormatInfo.InvariantInfo); //Create the old file name and checking if exists, if exists delete it var oldFileName = CombinePath(File, oldDateFormat); if (FileExists(oldFileName)) DeleteFile(oldFileName); //we are done, break it break; } } } ... }
          Hide
          Dominik Psenner added a comment - - edited

          Hi Jochen. I just copied your RollingFileAppender.cs over the latest in SVN and see lots of differences. For example the DateTimeStrategy property is missing and some other things don't look sensible. Like this I cannot apply your file as it would break other things. But if you could integrate your patch into the latest trunk I gladly review it.

          Show
          Dominik Psenner added a comment - - edited Hi Jochen. I just copied your RollingFileAppender.cs over the latest in SVN and see lots of differences. For example the DateTimeStrategy property is missing and some other things don't look sensible. Like this I cannot apply your file as it would break other things. But if you could integrate your patch into the latest trunk I gladly review it.
          Hide
          Jochen Kühner added a comment -

          I use my Patched Version of Log4Net on 5 Costomers now for over 6 Moth without any Problem.
          Is this patch now in the Main Branch?

          Show
          Jochen Kühner added a comment - I use my Patched Version of Log4Net on 5 Costomers now for over 6 Moth without any Problem. Is this patch now in the Main Branch?
          Hide
          Kamen Dimitrov added a comment -

          I did download the source and looked at all the attached files and took the source from there, and build the project. I took the log4net.dll, and I've tried it locally to see if it is going to work, but it didn't, so it might be a problem with the source that I build or it might be misconfiguration on my side. Unfortunately because I'm not sure how should the config file that is using the log4net.dll I can't confirm anything, that's why I was wondering if someone has made it work, so he can upload all the files.

          Show
          Kamen Dimitrov added a comment - I did download the source and looked at all the attached files and took the source from there, and build the project. I took the log4net.dll, and I've tried it locally to see if it is going to work, but it didn't, so it might be a problem with the source that I build or it might be misconfiguration on my side. Unfortunately because I'm not sure how should the config file that is using the log4net.dll I can't confirm anything, that's why I was wondering if someone has made it work, so he can upload all the files.
          Hide
          Dominik Psenner added a comment - - edited

          Are You able to build log4net from source? If so You are hereby invited to test the patches attached to this issue and maybe one of them implements Your usecase and once You report back the results, I am willing to apply and commit a patch that You confirm to be good.

          Show
          Dominik Psenner added a comment - - edited Are You able to build log4net from source? If so You are hereby invited to test the patches attached to this issue and maybe one of them implements Your usecase and once You report back the results, I am willing to apply and commit a patch that You confirm to be good.
          Hide
          Kamen Dimitrov added a comment - - edited

          I didn't express myself correctly, I am talking about the "maxDateRollBackups", which If I understand correctly is supposed to be used for specifying how many days you want to keep your logs

          Show
          Kamen Dimitrov added a comment - - edited I didn't express myself correctly, I am talking about the "maxDateRollBackups", which If I understand correctly is supposed to be used for specifying how many days you want to keep your logs
          Hide
          Dominik Psenner added a comment -

          There is no patch that mentions "MaxNumberOfDays" - am I blind or just dumb?

          Show
          Dominik Psenner added a comment - There is no patch that mentions "MaxNumberOfDays" - am I blind or just dumb?
          Hide
          Kamen Dimitrov added a comment -

          I was talking about the code for the MaxNumberOfDays like where to put code from the patch which will enable the functionality, anyways I think I figured it out. So now it will be great if somebody post a file with an example configuration, where the new functionality is used

          Show
          Kamen Dimitrov added a comment - I was talking about the code for the MaxNumberOfDays like where to put code from the patch which will enable the functionality, anyways I think I figured it out. So now it will be great if somebody post a file with an example configuration, where the new functionality is used
          Hide
          Dominik Psenner added a comment -

          Which changes would that be? Please be more precise. Otherwise You will have to wait for the next release that maybe will include a rewrite of the RollingFileAppender.

          Show
          Dominik Psenner added a comment - Which changes would that be? Please be more precise. Otherwise You will have to wait for the next release that maybe will include a rewrite of the RollingFileAppender.
          Hide
          Kamen Dimitrov added a comment -

          Hello, can somebody post the RollingFileAppender.cs with the implemented changes for the MaxNumberOfDays?

          Show
          Kamen Dimitrov added a comment - Hello, can somebody post the RollingFileAppender.cs with the implemented changes for the MaxNumberOfDays?
          Hide
          Joshua Masek added a comment -

          >>Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release?

          I can do that too.

          I wish I could donate my time to helping with fixing or rewriting RollingFileAppender, but can't for the foreseeable future...

          Show
          Joshua Masek added a comment - >>Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release? I can do that too. I wish I could donate my time to helping with fixing or rewriting RollingFileAppender, but can't for the foreseeable future...
          Hide
          Piotr Jastrząbek added a comment -

          >>Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release?

          I can do that.

          Guys?

          Show
          Piotr Jastrząbek added a comment - >>Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release? I can do that. Guys?
          Hide
          Stefan Bodewig added a comment -

          One problem I see is that the file added 26/Apr/11 is not licensed to the ASF, so I must not use it to patch trunk. This leads to the unfortunate situation where I'll have to evaluate Jochen's patch of Feb 2012 whether it seems to be based on the Apr 2011 version - or just the older patches. This sounds messy and probably is, but the ASF certainly will not publish code of other people who don't allow it to do.

          Show
          Stefan Bodewig added a comment - One problem I see is that the file added 26/Apr/11 is not licensed to the ASF, so I must not use it to patch trunk. This leads to the unfortunate situation where I'll have to evaluate Jochen's patch of Feb 2012 whether it seems to be based on the Apr 2011 version - or just the older patches. This sounds messy and probably is, but the ASF certainly will not publish code of other people who don't allow it to do.
          Hide
          Stefan Bodewig added a comment - - edited

          This issue may be the one with the most votes, but if you search around JIRA you'll see RollingFileAppender is broken in so many ways. I think it is responsible for about a third of all reports.

          Last summer when we waded through the issues to triage what should go into 1.2.11 I deliberately left out all RFA issues as - to be honest - I don't think it can be fixed without rewriting it. Back then some people started to work on a new implementation so this decision seemed to be a good one.

          Unfortunately life has interfered with the efforts of back then, as with the time I can devote to log4net myself. I'll look into the latest incarnation of this patch but honestly would prefer a re-thought and re-written RFA myself.

          Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release?

          Show
          Stefan Bodewig added a comment - - edited This issue may be the one with the most votes, but if you search around JIRA you'll see RollingFileAppender is broken in so many ways. I think it is responsible for about a third of all reports. Last summer when we waded through the issues to triage what should go into 1.2.11 I deliberately left out all RFA issues as - to be honest - I don't think it can be fixed without rewriting it. Back then some people started to work on a new implementation so this decision seemed to be a good one. Unfortunately life has interfered with the efforts of back then, as with the time I can devote to log4net myself. I'll look into the latest incarnation of this patch but honestly would prefer a re-thought and re-written RFA myself. Are there enough people watching this issue who would actually be able to build log4net from svn trunk to verify it fixes their problem (and doesn't introduce new ones) before I'd cut a new release?
          Hide
          Joshua Masek added a comment -

          What I originally submitted (and looks like it has been made into a patch), I have been running in production for close to 3 years (against the 1.2.10 version), and am not aware of any issues with it. Seems to be working, and nobody has had a harddisk fill up with logs (and I log heavily).
          I don't know if there are any concerns that have prevented it being in the trunk. My guess is that development on log4net has been minimal (considering it took 5 years to have a new minor version release), and may have something to do with this. Also in my opinion, it isn't simple to resolve cleanly (I did my best) and maybe there is still thought to be put into it.
          What I provided wasn't necessarily even me recommending it to be in the trunk, but rather just something people could use to get around the limitation.

          Show
          Joshua Masek added a comment - What I originally submitted (and looks like it has been made into a patch), I have been running in production for close to 3 years (against the 1.2.10 version), and am not aware of any issues with it. Seems to be working, and nobody has had a harddisk fill up with logs (and I log heavily). I don't know if there are any concerns that have prevented it being in the trunk. My guess is that development on log4net has been minimal (considering it took 5 years to have a new minor version release), and may have something to do with this. Also in my opinion, it isn't simple to resolve cleanly (I did my best) and maybe there is still thought to be put into it. What I provided wasn't necessarily even me recommending it to be in the trunk, but rather just something people could use to get around the limitation.
          Hide
          Piotr Jastrząbek added a comment -

          This is the most popular issue right now according to
          https://issues.apache.org/jira/browse/LOG4NET#selectedTab=com.atlassian.jira.plugin.system.project%3Apopularissues-panel

          Guys, have you been using patched version on production?
          Is sth. wrong with this patch that it still not present in trunk?

          Show
          Piotr Jastrząbek added a comment - This is the most popular issue right now according to https://issues.apache.org/jira/browse/LOG4NET#selectedTab=com.atlassian.jira.plugin.system.project%3Apopularissues-panel Guys, have you been using patched version on production? Is sth. wrong with this patch that it still not present in trunk?
          Hide
          Jochen Kühner added a comment -

          Patched the patch to work with the newest source, and changed the file deletion a little bit (I use the lastwritedate to find the oldest file!)

          Show
          Jochen Kühner added a comment - Patched the patch to work with the newest source, and changed the file deletion a little bit (I use the lastwritedate to find the oldest file!)
          Hide
          Lieven Cardoen added a comment -

          Does this work now in the source control current revision version?

          Show
          Lieven Cardoen added a comment - Does this work now in the source control current revision version?
          Hide
          David Imboden added a comment -

          Will be very important for CompactFramework, mobile computers do not have much capacity.

          Show
          David Imboden added a comment - Will be very important for CompactFramework, mobile computers do not have much capacity.
          Hide
          Petr Felzmann added a comment - - edited

          I created patch from Joshua Masek changes against the current revision in source control. Hope it moves this issue little bit forward.

          Personally I'm still on the way to understand the code to be able implement ideas from Ron's comment (05/Jun/09 10:28).

          Also I'm very surprised how the unit tests are complicated. I'd like to write unit tests for this change... but there are missing completely unit tests for anything concerning date/time

          Show
          Petr Felzmann added a comment - - edited I created patch from Joshua Masek changes against the current revision in source control. Hope it moves this issue little bit forward. Personally I'm still on the way to understand the code to be able implement ideas from Ron's comment (05/Jun/09 10:28). Also I'm very surprised how the unit tests are complicated. I'd like to write unit tests for this change... but there are missing completely unit tests for anything concerning date/time
          Hide
          David Juffermans added a comment -

          So is this issue ever going to be fixed or what?

          Show
          David Juffermans added a comment - So is this issue ever going to be fixed or what?
          Hide
          Joshua Masek added a comment -

          Yeah, I'm not sure I implemented it the best way. It was more of a workaround against the current release until similar functionality makes it into the official build, and I posted it at least to make it available to others that could use the functionality now.

          When I get more time (might have to be weeks or months), I might have a chance to make a better patch and base it on the newest revision.

          As for the digits like .10, .11., etc. Its looping thru the characters from the end of the string looking for either digits or a period. At the time, that was the way it occurred to check if the char was a digit. (Now, I realize I could simply do something like >= "0" and <= "9" or something. But, I didn't test with more than 4... so, I should test that.

          When rolling sizes, then the files extensions (.1, .2, .3, etc) are getting renamed. When rolling dates, they aren't, so there isn't any previous file identified.

          Actually, there was some code that looked at the system's date modified time, although in a different function in that class.

          Your last comment about your idea:
          The problem I had was this:
          (well, first, files are rolled/renamed in the size functionality, not the date functionality, except for the initial rename from the basefilename to the filename with the date format embedded)
          there could be files sitting there that need deleted from a lot longer ago than the MaxDateRollBackups value allows, and these files need deleted (they still exist because, say, the program hasn't been in awhile). So, somehow those files have to be identified. They have the formatted date inside of it. How to find all file with the base filename and a date formatted with that format (and also potentially any size extensions on them)? Some date formats can be difficult to parse from the string back into the date. Especially the ones that spit out long names and such. They can throw all kind of crazy formats into those files. Using a basic * wildcard works easily, but could cause files to be deleted that aren't desired to be deleted. LIke if someone renamed it manually with a descriptive extention to save the file. I'm not sure if it is wise, but to be safe and not delete files that shouldn't be deleted, I figured looping thru the files based on the simple * wildcard, skipping the ones we know are still good within the MaxDateRollBackups and MaxSizeRollBackups limits, we can take the rest, get each ones' modified timestamp, format that timestamp, compare against the formatted timestamp in the filename, if it matches exactly, then it hasn't been renamed, and can be deleted. Also, check the previous check timestamp prior to the modified timestamp, because the file could have been modified as it was crossing the boundary.

          I'm just not sure if there are any reasons not to trust the modified timestamp. But I didn't know how else to successfully remove those files, and not files that I renamed.

          As a note, my method using modified timestamps may not work well if the date format has seconds or milliseconds in it.

          I'm not suggesting log4net do it that way, its just the only way I could do it and correctly have it decide to delete or not delete each file.

          Show
          Joshua Masek added a comment - Yeah, I'm not sure I implemented it the best way. It was more of a workaround against the current release until similar functionality makes it into the official build, and I posted it at least to make it available to others that could use the functionality now. When I get more time (might have to be weeks or months), I might have a chance to make a better patch and base it on the newest revision. As for the digits like .10, .11., etc. Its looping thru the characters from the end of the string looking for either digits or a period. At the time, that was the way it occurred to check if the char was a digit. (Now, I realize I could simply do something like >= "0" and <= "9" or something. But, I didn't test with more than 4... so, I should test that. When rolling sizes, then the files extensions (.1, .2, .3, etc) are getting renamed. When rolling dates, they aren't, so there isn't any previous file identified. Actually, there was some code that looked at the system's date modified time, although in a different function in that class. Your last comment about your idea: The problem I had was this: (well, first, files are rolled/renamed in the size functionality, not the date functionality, except for the initial rename from the basefilename to the filename with the date format embedded) there could be files sitting there that need deleted from a lot longer ago than the MaxDateRollBackups value allows, and these files need deleted (they still exist because, say, the program hasn't been in awhile). So, somehow those files have to be identified. They have the formatted date inside of it. How to find all file with the base filename and a date formatted with that format (and also potentially any size extensions on them)? Some date formats can be difficult to parse from the string back into the date. Especially the ones that spit out long names and such. They can throw all kind of crazy formats into those files. Using a basic * wildcard works easily, but could cause files to be deleted that aren't desired to be deleted. LIke if someone renamed it manually with a descriptive extention to save the file. I'm not sure if it is wise, but to be safe and not delete files that shouldn't be deleted, I figured looping thru the files based on the simple * wildcard, skipping the ones we know are still good within the MaxDateRollBackups and MaxSizeRollBackups limits, we can take the rest, get each ones' modified timestamp, format that timestamp, compare against the formatted timestamp in the filename, if it matches exactly, then it hasn't been renamed, and can be deleted. Also, check the previous check timestamp prior to the modified timestamp, because the file could have been modified as it was crossing the boundary. I'm just not sure if there are any reasons not to trust the modified timestamp. But I didn't know how else to successfully remove those files, and not files that I renamed. As a note, my method using modified timestamps may not work well if the date format has seconds or milliseconds in it. I'm not suggesting log4net do it that way, its just the only way I could do it and correctly have it decide to delete or not delete each file.
          Hide
          Ron Grabowski added a comment -

          Thanks for the patch! Is there anyway you can write the patch against the current revision in source control? There's been a number of largish pages since the 1.2.10 release. I haven't diff'ed the two files yet...maybe its just a simple copy paste of one or two methods and fixing wrapping the file IO calls to be called within the correct context like the calls to File.Move and File.Delete.

          I'm not sure I follow this code:

          else if (!byte.TryParse(current.Substring(charLoop, 1), out dummyByte))

          Will that work if the numbers are not single digits: 10, .11, .12 ?

          I don't understand the need for PreviousCheckDate. Can't you determine the previous and current files by capturing the parameters to RollFile?

          None of the other file rolling code looks at the file system's date modified time...its all based on the filename. I wonder if this inconsistency will cause unexpected behavior.

          My idea was to change the signature of RollFile to return the name of the rolled file, create a list of rolled files, then iterate through that list and determine which of the new files are over the MaxDateRollBackups threshold and delete those files. I don't think it should take a lot of code to implement this functionality.

          Show
          Ron Grabowski added a comment - Thanks for the patch! Is there anyway you can write the patch against the current revision in source control? There's been a number of largish pages since the 1.2.10 release. I haven't diff'ed the two files yet...maybe its just a simple copy paste of one or two methods and fixing wrapping the file IO calls to be called within the correct context like the calls to File.Move and File.Delete. I'm not sure I follow this code: else if (!byte.TryParse(current.Substring(charLoop, 1), out dummyByte)) Will that work if the numbers are not single digits: 10, .11, .12 ? I don't understand the need for PreviousCheckDate. Can't you determine the previous and current files by capturing the parameters to RollFile? None of the other file rolling code looks at the file system's date modified time...its all based on the filename. I wonder if this inconsistency will cause unexpected behavior. My idea was to change the signature of RollFile to return the name of the rolled file, create a list of rolled files, then iterate through that list and determine which of the new files are over the MaxDateRollBackups threshold and delete those files. I don't think it should take a lot of code to implement this functionality.
          Hide
          Joshua Masek added a comment -

          I made the change for myself so that I could have the functionality now. I tried to keep the change compatible with the other options:

          • I started with the RollingFileAppender.cs.patch
          • I moved the logic into RollOverTime
          • I created a new property: MaxDateRollBackups
          • I made sure it deleted log files with combination of date and rolled by size
          • I don't delete files where the filename doesn't match the last modified timestamp or the previous check timestamp prior to the last modified timestamp. This was so that it would not delete files like logfile.txt2005-07-22.txt.1.keepThisForFutureReference
          • I made sure it works for when staticLogFileName is true or false
          • I'm attaching my version of the RollingFileAppender.cs file. It was based of the v1.2.10.0 build.
          Show
          Joshua Masek added a comment - I made the change for myself so that I could have the functionality now. I tried to keep the change compatible with the other options: I started with the RollingFileAppender.cs.patch I moved the logic into RollOverTime I created a new property: MaxDateRollBackups I made sure it deleted log files with combination of date and rolled by size I don't delete files where the filename doesn't match the last modified timestamp or the previous check timestamp prior to the last modified timestamp. This was so that it would not delete files like logfile.txt2005-07-22.txt.1.keepThisForFutureReference I made sure it works for when staticLogFileName is true or false I'm attaching my version of the RollingFileAppender.cs file. It was based of the v1.2.10.0 build.
          Hide
          Joshua Masek added a comment -

          I'm surprised this feature is still missing. Automatically rolling and deleting old logs by date is often the preferred method asked of me.

          Show
          Joshua Masek added a comment - I'm surprised this feature is still missing. Automatically rolling and deleting old logs by date is often the preferred method asked of me.
          Hide
          David Juffermans added a comment -

          Unbelievable this isn't in there yet. So for almost 4 years already people using a (daily) rolling file appender have to clean up log files manually?

          Show
          David Juffermans added a comment - Unbelievable this isn't in there yet. So for almost 4 years already people using a (daily) rolling file appender have to clean up log files manually?
          Hide
          Nicko Cadell added a comment -

          I think that this date period cleanup logic should go in the RollOverTime method rather than OpenFile.

          Rather than reusing MaxSizeRollBackups I think that we need another property, MaxDatePeriods, to control the number of date periods archived. This is because when RollingStyle=Composite the output is split into date periods, but within each period the files are size rolled and the MaxSizeRollBackups property is used to control how many size rolled files should be kept per date period. As we would want to be able to combine the number of date periods with the number of size rolled files we would need another property to do this.

          Because of the composite mode it is also necessary to delete any size rolled files within the date period. These would be called logfile.txt2005-07-22.txt.1, logfile.txt2005-07-22.txt.2 etc... i.e. they have a numbered postfix on the file name.

          Show
          Nicko Cadell added a comment - I think that this date period cleanup logic should go in the RollOverTime method rather than OpenFile. Rather than reusing MaxSizeRollBackups I think that we need another property, MaxDatePeriods, to control the number of date periods archived. This is because when RollingStyle=Composite the output is split into date periods, but within each period the files are size rolled and the MaxSizeRollBackups property is used to control how many size rolled files should be kept per date period. As we would want to be able to combine the number of date periods with the number of size rolled files we would need another property to do this. Because of the composite mode it is also necessary to delete any size rolled files within the date period. These would be called logfile.txt2005-07-22.txt.1, logfile.txt2005-07-22.txt.2 etc... i.e. they have a numbered postfix on the file name.
          Hide
          Florian Ramillien added a comment -

          More appropriate patch

          • Now work with rolling other than by Day
          • based on CVS, not last beta

          But : Logic to delete file in 'OpenFile' method, is it the good place ?

          Show
          Florian Ramillien added a comment - More appropriate patch Now work with rolling other than by Day based on CVS, not last beta But : Logic to delete file in 'OpenFile' method, is it the good place ?
          Hide
          Florian Ramillien added a comment -

          On 'OpenFile', build all valid backup file names and delete all backup files that don't match this list.

          Show
          Florian Ramillien added a comment - On 'OpenFile', build all valid backup file names and delete all backup files that don't match this list.

            People

            • Assignee:
              Unassigned
              Reporter:
              Florian Ramillien
            • Votes:
              10 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:

                Development