Allowing (again) the rotation of files that haven't been written to.

Rcomian · Feb 27, 2016 · 82cc8c5 · 82cc8c5
1 parent 55fdc48
commit 82cc8c5
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 8 deletions.
diff --git a/lib/limitedqueue.js b/lib/limitedqueue.js
@@ -112,6 +112,10 @@ function LimitedQueue(worker) {
         }
     }
 
+    function isEmpty() {
+        return queue.length() === 0;
+    }
+
     function paused() {
         return queue.paused;
     }
@@ -130,7 +134,8 @@ function LimitedQueue(worker) {
         pause: queue.pause,
         resume: queue.resume,
         unshift: queue.unshift,
-        join
+        join,
+        isEmpty
     }, base);
 }
 

diff --git a/lib/rotatingfilestream.js b/lib/rotatingfilestream.js
@@ -169,16 +169,34 @@ function RotatingFileStream(options) {
 
     function rotateActual() {
 
-        base.once('data', function () {
-            rotateFunction = rotateActual;
-        });
-
         rotateFunction = function () {};
 
-        rotator.rotate(function (err, newstream, filePath) {
+        async.parallel([
+            function waitForWrite(next) {
+                rotator.once('newfile', function () {
+                    if (writeQueue.isEmpty()) {
+                        // No logs to write, so we're all clear to allow
+                        // rotations again
+                        next();
+                    } else {
+                        // We've got some logs to write, ensure we get at least
+                        // one log record into the file before allowing
+                        // another rotation
+                        base.once('data', function (info) {
+                            next();
+                        });
+                    }
+                });
+            },
+            function doRotation(next) {
+                rotator.rotate(next);
+            }
+        ], function allowRotationsAgain(err) {
             if (err) {
                 base.emit('error', err);
             }
+
+            rotateFunction = rotateActual;
         });
     }
 

diff --git a/test-perf.js b/test-perf.js
@@ -83,7 +83,7 @@ function throughput(next) {
         function (next) { rmdir(name, ignoreMissing(next)); },
         function (next) { fx.mkdir(name, next); },
         function (next) { runTest (name, {
-            stream: { path: name + '/test.log' },
+            stream: { path: name + '/test.log', noCyclesCheck: true },
             batch: { iterations: 1000000, size: 1000 }
         }, next); },
         function (next) {

diff --git a/test.js b/test.js
@@ -23,6 +23,7 @@ function runTest(name, options, next) {
     var log = bunyan.createLogger({
         name: 'foo',
         streams: [{
+            type: 'raw',
             stream: rfs
         }]
     });
@@ -84,7 +85,7 @@ function basicthreshold(next) {
         function (next) { rmdir(name, ignoreMissing(next)); },
         function (next) { fx.mkdir(name, next); },
         function (next) { runTest (name, {
-            stream: { path: name + '/test.log', threshold: '1m' },
+            stream: { path: name + '/test.log', threshold: '1m', fieldOrder: ['pid', 'time'] },
             batch: { iterations: 100000 }
         }, next); },
         function (next) {
@@ -137,6 +138,26 @@ function timerotation(next) {
     ], next);
 }
 
+function timerotationnologging(next) {
+    var name = 'testlogs/' + 'timerotationnologging';
+
+    async.series([
+        function (next) { rmdir(name, ignoreMissing(next)); },
+        function (next) { fx.mkdir(name, next); },
+        function (next) { runTest (name, {
+            stream: { path: name + '/test.log', period: '1000ms' },
+            batch: { size: 0, duration: 9500 }
+        }, next); },
+        function (next) {
+            var files = fs.readdirSync(name);
+            assert.equal(10, files.length);
+            console.log(name, 'passed');
+            next();
+        },
+        function (next) { rmdir(name, ignoreMissing(next)); }
+    ], next);
+}
+
 function gzippedfiles(next) {
     var name = 'testlogs/' + 'gzippedfiles';
 
@@ -362,6 +383,7 @@ function checksetlongtimeoutclearnormalperiods(next) {
 async.parallel([
     basicthreshold,
     timerotation,
+    timerotationnologging,
     gzippedfiles,
     totalsize,
     totalfiles,