Use temporary file when compressing rotated logs and atomically renam…

…e to prevent reading incomplete files

If another process is watching for `*.gz` files then it's possible to
begin reading the archive before it has been completely created,
resulting in corruption if the other process is copying the archive to
another location (for example: archival to s3).

To resolve this, we can use a different suffix when writing the file so
that other programs do not read it while it's being created. Once the
archive has been completely created, we atomically rename it to the
desired file name with the `*.gz` extension, ensuring external programs
only ever see the finished archive.

Signed-off-by: Chance Zibolski <[email protected]>

lumberjack.go

@@ -38,6 +38,7 @@ import (
 const (
 	backupTimeFormat = "2006-01-02T15-04-05.000"
 	compressSuffix   = ".gz"
+	tmpSuffix        = ".tmp"
 	defaultMaxSize   = 100
 )
 
@@ -477,13 +478,17 @@ func compressLogFile(src, dst string) (err error) {
 		return fmt.Errorf("failed to stat log file: %v", err)
 	}
 
-	if err := chown(dst, fi); err != nil {
+	// Use a different filename to write the file, so that anything looking for
+	// "*.gz" only sees the compressed file after it's been finished writing to.
+	tmpDst := dst + tmpSuffix
+
+	if err := chown(tmpDst, fi); err != nil {
 		return fmt.Errorf("failed to chown compressed log file: %v", err)
 	}
 
 	// If this file already exists, we presume it was created by
 	// a previous attempt to compress the log file.
-	gzf, err := os.OpenFile(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, fi.Mode())
+	gzf, err := os.OpenFile(tmpDst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, fi.Mode())
 	if err != nil {
 		return fmt.Errorf("failed to open compressed log file: %v", err)
 	}
@@ -493,14 +498,20 @@ func compressLogFile(src, dst string) (err error) {
 
 	defer func() {
 		if err != nil {
-			os.Remove(dst)
+			os.Remove(tmpDst)
 			err = fmt.Errorf("failed to compress log file: %v", err)
 		}
 	}()
 
 	if _, err := io.Copy(gz, f); err != nil {
 		return err
 	}
+
+	// fsync is important, otherwise os.Rename could rename a zero-length file
+	if err := gzf.Sync(); err != nil {
+		return err
+	}
+
 	if err := gz.Close(); err != nil {
 		return err
 	}
@@ -511,6 +522,12 @@ func compressLogFile(src, dst string) (err error) {
 	if err := f.Close(); err != nil {
 		return err
 	}
+
+	// Atomically replace the destination file
+	if err := os.Rename(tmpDst, dst); err != nil {
+		return err
+	}
+
 	if err := os.Remove(src); err != nil {
 		return err
 	}