Skip to content

Commit

Permalink
Fix logfile tailing under Node 10.16.0
Browse files Browse the repository at this point in the history
`fs.watch` does not consistently emit `'change'` events on Node 10.16.0,
possibly due to nodejs/node#29460. `fs.watchFile`
still works, but is less efficient, per the docs, so fall back to that only if necessary and also suggest that users run custody under an older version of Node.

I explored several Node file-watching libraries (e.g. chokidar, sane) to try to
find a more performant solution for Node 10.16.0 and it seems that they pretty
much all fall back to `fs.watch` / `fs.watchFile`, unfortunately. sane offers
the option to use watchman, but that requires installing a daemon so is a
non-starter.

Fixes #82.

#83 tracks reverting this when
the underlying Node issue has been fixed.

Tested that log files update both when running custody under v8.9.3 and
v10.16.0, and that `node-tail` only uses `fs.watchFile` in the latter.
  • Loading branch information
wearhere committed Sep 19, 2019
1 parent 7a4682c commit 5caea0c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 5 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ to set it.

## Usage

> Node 10.16.0 shipped [a regression](https://github.com/mixmaxhq/custody/issues/82#issuecomment-533348756)
in file watching that may degrade performance when custody is tailing log files. If you experience
high CPU utilization while using custody, please try running custody under an older version of Node
e.g. using [`nvm`](https://github.com/nvm-sh/nvm).

Make sure that Supervisor is running. Then run

```sh
Expand Down
33 changes: 29 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"react-blessed": "^0.3.0",
"readline-sync": "^1.4.9",
"sanitize-filename": "^1.6.1",
"semver": "^6.3.0",
"supervisord": "^0.1.0",
"underscore": "^1.9.0",
"yargs": "^11.0.0"
Expand Down
25 changes: 24 additions & 1 deletion src/components/FileLog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import PropTypes from 'prop-types';
import React, {Component} from 'react';
import {Tail} from '@mixmaxhq/tail';
import screen, {enableMouse} from '/screen';
import * as semver from 'semver';
import {statSync} from 'fs';

// It might be nice to render the entire log file. However this is probably (?) unnecessary and
Expand Down Expand Up @@ -54,11 +55,33 @@ export default class FileLog extends Component {
return;
}

/**
* `fs.watch` does not consistently emit 'change' events when data is written to the log files
* under Node 10.16.0, possibly due to https://github.com/nodejs/node/issues/29460.
* `fs.watchFile` still works, but is less efficient, per the docs, so only use it if necessary
* and also suggest that users run custody under an older version of Node. See
* https://github.com/mixmaxhq/custody/issues/82 for more information.
*/
const mustUseWatchFile = semver.gte(process.version, '10.16.0');
if (mustUseWatchFile) {
screen.debug('WARNING: `fs.watch` is broken when using this version of Node, falling back ' +
'to `fs.watchFile`. To reduce CPU utilization, run `custody-cli` under an older version ' +
'of Node. For more information see https://github.com/mixmaxhq/custody/issues/82.');
}

// As documented on `INITIAL_SCROLLBACK`, we can't render the entire log file. However, the
// 'tail' module lacks a `-n`-like option to get the last `INITIAL_SCROLLBACK` lines. So what we
// do is load the entire file, but wait to render only the last `INITIAL_SCROLLBACK` lines, then
// start streaming.
this.tail = new Tail(logfile, { fromBeginning: true });
this.tail = new Tail(logfile, {
fromBeginning: true,
...(mustUseWatchFile && {
useWatchFile: true,
fsWatchOptions: {
interval: 100,
}
})
});

let logs = [];
let initialDataFlushed = false;
Expand Down

0 comments on commit 5caea0c

Please sign in to comment.