Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable hooks in fs.statWatchers #33569

Closed
himself65 opened this issue May 26, 2020 · 8 comments
Closed

Enable hooks in fs.statWatchers #33569

himself65 opened this issue May 26, 2020 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. stale

Comments

@himself65
Copy link
Member

himself65 commented May 26, 2020

Is your feature request related to a problem? Please describe.
People sometimes will encounter the error that System limit for number of file watchers reached.. If we enable users to add hooks in fs.statWatchers, they could track the codes that which module call fs.watchFile many times

Describe the solution you'd like

const { statWatchers, watchFile } = require(‘fs’)
statWatchers.on(‘statAdd’, () => {})
statWatchers.on(‘statRemove’, () => {})

watchFile(‘xxx‘, () => {})

Related code

node/lib/fs.js

Line 1458 in 9949a2e

const statWatchers = new Map();

@himself65 himself65 added the feature request Issues that request new features to be added to Node.js. label May 26, 2020
@Zheaoli
Copy link

Zheaoli commented May 26, 2020

I think this feature would be necessary

In Linux or other platforms, the file watcher is very expensive resources for user. For example, there are some limit about file watcher count for the unique user id in Linux

So I think it would be necessary to allow people get a way to trace/output log/do other things meaningful for their 'file watch' action

@addaleax
Copy link
Member

If we enable users to add hooks in fs.statWatchers, they could track the codes that which module call fs.watchFile many times

Two questions:

  • Doesn’t this happen with fs.watch() only? The stat watchers shouldn’t count towards a global limit?
  • Doesn’t it suffice to override/wrap fs.watch() and fs.unwatch() if this is what you’re looking for?

@addaleax addaleax added the fs Issues and PRs related to the fs subsystem / file system. label May 26, 2020
@himself65
Copy link
Member Author

Doesn’t this happen with fs.watch() only? The stat watchers shouldn’t count towards a global limit?

Yes, it do is the global limit, but sometimes we need to monitor which module reach the limit.

Doesn’t it suffice to override/wrap fs.watch() and fs.unwatch() if this is what you’re looking for?

I noticed that we already have fs. statWatchers using Map, to memory the stats. What about we enhance it to the EventEmitter or anything else, not just a third part module to achieve this.

@addaleax
Copy link
Member

Doesn’t this happen with fs.watch() only? The stat watchers shouldn’t count towards a global limit?

Yes, it do is the global limit, but sometimes we need to monitor which module reach the limit.

I don’t really understand how this relates to the difference between fs.watch() and fs.watchFile()? fs.watchFile() does not take up OS resources, as I understand it.

Doesn’t it suffice to override/wrap fs.watch() and fs.unwatch() if this is what you’re looking for?

I noticed that we already have fs. statWatchers using Map, to memory the stats. What about we enhance it to the EventEmitter or anything else, not just a third part module to achieve this.

I guess to be more explicit, what is the advantage of using an EventEmitter here, as opposed to keeping the API surface as it is?

@jasnell
Copy link
Member

jasnell commented Apr 28, 2021

Just getting back around on this issue. I don't know if it's still something that is needed, but it should be possible to do what you want here (or get close) using an async_hook:

const { createHook } = require('async_hooks');

const set = new Set();

const hook = createHook({
  init(id, type) {
    if (type === 'FSEVENTWRAP') set.add(id);
  },
  destroy(id) {
    set.delete(id);
  }
});

hook.enable();

const { watch } = require('fs');

// The size is the number of active watchers...
setInterval(() => console.log(set.size), 1000);

let w;

setTimeout(() => w = watch(__filename), 500);

setTimeout(() => w.close(), 1500);

It can be expensive so you wouldn't want to have this enabled all the time, but to determine which code path created the watch, you should be able to do something like:

const map = new Map();

const hook = createHook({
  init(id, type) {
    if (type === 'FSEVENTWRAP') {
      const err = new Error();
      map.put(id, err.stack);
    }
  },
  destroy(id) {
    set.delete(id);
  }
});

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 24, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. stale
Projects
Development

No branches or pull requests

4 participants