Rewrote log file rotation to use just one thread for zipping all log files.

src/helma/util/FileLogger.java

@@ -21,7 +21,6 @@ import org.apache.commons.logging.Log;
 import java.io.*;
 import java.text.*;
 import java.util.*;
-import java.util.zip.GZIPOutputStream;
 
 /**
  * An extended Logger that writes to a file and rotates files each midnight.
@@ -77,12 +76,15 @@ public class FileLogger extends Logger implements Log {
         try {
             if (logfile.exists() && (logfile.lastModified() < lastMidnight())) {
                 // rotate if a log file exists and is NOT from today
-                rotateLogFile();
+                File archive = rotateLogFile();
-            } else {
+                // gzip rotated log file in a separate thread
+                if (archive != null) {
+                    new Logging.FileGZipper(archive).start();
+                }
+            }
             // create a new log file, appending to an existing file
             writer = new PrintWriter(new FileWriter(logfile.getAbsolutePath(), true),
                                      false);
-            }
         } catch (IOException iox) {
             System.err.println("Error creating log " + name + ": " + iox);
         }
@@ -102,13 +104,6 @@ public class FileLogger extends Logger implements Log {
         }
     }
 
-    /**
-     *  Return true if we have a file writer open
-     */
-    synchronized boolean isOpen() {
-        return writer != null;
-    }
-
     /**
      * This is called by the runner thread to perform actual output.
      */
@@ -147,49 +142,51 @@ public class FileLogger extends Logger implements Log {
 
 
     /**
-     *  Rotate log files, closing, renaming and gzipping the old file and
+     *  Rotate log files, closing the file writer and renaming the old
-     *  start a new one.
+     *  log file. Returns the renamed log file for zipping, or null if
+     *  the log file couldn't be rotated.
+     *
+     *  @return the old renamed log file, or null
      */
-    protected synchronized void rotateLogFile() throws IOException {
+    protected synchronized File rotateLogFile() throws IOException {
         // if the logger is not file based do nothing.
         if (logfile == null) {
-            return;
+            return null;
         }
 
-        if (writer != null) {
+        closeFile();
-            try {
-                writer.close();
-            } catch (Exception ignore) {
-            }
-        }
 
         // only backup/rotate if the log file is not empty,
         if (logfile.exists() && (logfile.length() > 0)) {
             String today = aformat.format(new Date());
             int ct = 0;
-            File archive = null;
 
             // first append just the date
-            String archname = name + "-" + today + ".log.gz";
+            String archname = name + "-" + today + ".log";
-
+            File archive = new File(logdir, archname);
-            while ((archive == null) || archive.exists()) {
+            File zipped = new File(logdir, archname + ".gz");
-                archive = new File(logdir, archname);
 
+            // increase counter until we find an unused log archive name, checking
+            // both unzipped and zipped file names
+            while (archive.exists() || zipped.exists()) {
                 // for the next try we append a counter
                 String archidx = (ct > 999) ? Integer.toString(ct) : nformat.format(++ct);
 
-                archname = name + "-" + today + "-" + archidx + ".log.gz";
+                archname = name + "-" + today + "-" + archidx + ".log";
+                archive = new File(logdir, archname);
+                zipped = new File(logdir, archname + ".gz");
             }
 
             if (logfile.renameTo(archive)) {
-                (new GZipper(archive)).start();
+                return archive;
             } else {
                 System.err.println("Error rotating log file " + canonicalName +
-                        ". Old file will possibly be overwritten!");
+                        ". Will append to old file.");
             }
         }
 
-        writer = new PrintWriter(new FileWriter(logfile), false);
+        // no log file rotated
+        return null;
     }
 
     /**
@@ -255,39 +252,4 @@ public class FileLogger extends Logger implements Log {
         super.fatal(parm1, parm2);
     }
 
-    /**
-     * a Thread class that zips up a file, filename will stay the same.
-     */
-    class GZipper extends Thread {
-        File file;
-        File temp;
-        final static int BUFFER_SIZE = 8192;
-
-        public GZipper(File file) {
-            this.file = file;
-            this.temp = new File(file.getAbsolutePath() + ".tmp");
-            setPriority(MIN_PRIORITY);
-        }
-
-        public void run() {
-            try {
-                GZIPOutputStream zip = new GZIPOutputStream(new FileOutputStream(temp));
-                BufferedInputStream in = new BufferedInputStream(new FileInputStream(file));
-                byte[] b = new byte[BUFFER_SIZE];
-                int len = 0;
-
-                while ((len = in.read(b, 0, BUFFER_SIZE)) != -1) {
-                    zip.write(b, 0, len);
-                }
-
-                zip.close();
-                in.close();
-                file.delete();
-                temp.renameTo(file);
-            } catch (Exception e) {
-                System.err.println(e.toString());
-            }
-        }
-    }
-
 }

src/helma/util/Logging.java

@@ -19,6 +19,7 @@ package helma.util;
 import org.apache.commons.logging.*;
 import java.io.*;
 import java.util.*;
+import java.util.zip.GZIPOutputStream;
 
 /**
  *  Implementation of Jakarta Commons LogFactory that supports both
@@ -172,21 +173,26 @@ public class Logging extends LogFactory {
         consoleLog = null;
     }
 
-    /**
+    static void gzip(File file) {
-     * Rotate log files on all registered logs
+        final int BUFFER_SIZE = 8192;
-     */
-    static void rotateLogs() {
-        int nloggers = loggers.size();
-
-        for (int i = nloggers - 1; i >= 0; i--) {
-            FileLogger log = (FileLogger) loggers.get(i);
 
         try {
-                log.rotateLogFile();
+            File zipped = new File(file.getAbsolutePath() + ".gz");
-            } catch (IOException io) {
+            GZIPOutputStream zip = new GZIPOutputStream(new FileOutputStream(zipped));
-                System.err.println("Error rotating log " + log.getName() + ": " +
+            BufferedInputStream in = new BufferedInputStream(new FileInputStream(file));
-                                    io.toString());
+            byte[] b = new byte[BUFFER_SIZE];
+            int len = 0;
+
+            while ((len = in.read(b, 0, BUFFER_SIZE)) != -1) {
+                zip.write(b, 0, len);
             }
+
+            zip.close();
+            in.close();
+            file.delete();
+        } catch (Exception e) {
+            System.err.println("Error gzipping " + file);
+            System.err.println(e.toString());
         }
     }
 
@@ -224,7 +230,7 @@ public class Logging extends LogFactory {
                 long now = System.currentTimeMillis();
 
                 if (nextMidnight < now) {
-                    rotateLogs();
+                    new LogRotator().start();
                     nextMidnight = nextMidnight();
                 }
 
@@ -259,5 +265,57 @@ public class Logging extends LogFactory {
 
     }
 
+    /**
+     * Log rotator thread calls rotateLogFiles on all
+     */
+    static class LogRotator extends Thread {
+
+        public void run() {
+
+            FileLogger[] logs = (FileLogger[]) loggers.toArray(new FileLogger[0]);
+
+            ArrayList archives = new ArrayList();
+
+            for (int i = 0; i < logs.length; i++) {
+                try {
+                    File archive = logs[i].rotateLogFile();
+                    if (archive != null) {
+                        archives.add(archive);
+                    }
+                } catch (IOException io) {
+                    System.err.println("Error rotating log " + logs[i].getName() + ": " +
+                                        io.toString());
+                }
+            }
+
+            // reduce thread priority for zipping
+            setPriority(MIN_PRIORITY);
+            Iterator it = archives.iterator();
+            while (it.hasNext()) {
+                gzip((File) it.next());
+            }
+
+        }
+
+    }
+
+    /**
+     * Utility thread class to gzip a file in a separate thread
+     */
+    static class FileGZipper extends Thread {
+
+        File file;
+
+        FileGZipper(File file) {
+            this.file = file;
+        }
+
+        public void run() {
+            // reduce thread priority for zipping
+            setPriority(MIN_PRIORITY);
+            gzip(file);
+        }
+    }
+
 }