Avoid persistent mill goroutine

[jdhenke]

Previously, each lumberjack logger starts its own mill goroutine the first time that the logger writes to a file, and the goroutine is never stopped. The project has an open pull request, https://github.com/natefinch/lumberjack/pull/80, to stop the goroutine when the logger is closed. However, even with that design, each open logger will always have a mill goroutine that is active (even if it is not performing work). It also means that a logger will never be garbage-collected (and hence its file will never be closed) unless it is explicitly closed. Furthemore, even if a logger is explicitly closed, if Write is called on any references to that logger, that will restart the persistent mill goroutine and reintroduce the leaked file handle.

This change modifies the mill goroutine logic. It still maintains the overall design of synchronously rotating and asynchronously deleting and compressing (milling) the old, rotated log files, and that if a request to mill comes in while one call is already happening, another mill will be executed after the current one to ensure any newly rotated files are not missed. There is at most one active mill goroutine which is in the process of performing a single pass of deletions and compressions of rotated log files, and at most one queued mill goroutine which is just waiting on a mutex. Now, each mill goroutine exits once a single pass is done and future calls to mill will start a new goroutine (if one is not already queued.)

This modification allows for lumberjack loggers to be garbage-collected, which allows it to close file handles gracefully without an explicit call to "close".

The downside to this new approach is in the case where rotations are constantly happening, rather than having a single goroutine simply process each mill pass, a new goroutine must be started for each pass.

---

We have an internal fork of lumberjack that contains this change that we are using in production.

An alternative approach was sketched after the fact that is not in use internally but would have at most one mill goroutine per logger at any time, rather than one active and one queued as in this approach. This alternative design is shown here:

https://github.com/jdhenke/lumberjack/pull/2

[jdhenke]

FYI, looks like TestCompressOnRotate flakes.

These commits have the same diff:

• Fails - https://travis-ci.org/natefinch/lumberjack/jobs/568612017
• Passes - https://travis-ci.org/natefinch/lumberjack/builds/568612670

[natefinch]

Thanks, I'll take a look

[howbazaar]

One of the problems with this is that there is the possibility of having multiple calls to millRunOnce at one time. I worked on my own fix for this before I realised that there were previous PRs, #100.