Skip to content

use graceful-fs in place of fs to fix emfile errors#570

Merged
joehand merged 1 commit intomasterfrom
emfile-graceful-fs
Nov 16, 2016
Merged

use graceful-fs in place of fs to fix emfile errors#570
joehand merged 1 commit intomasterfrom
emfile-graceful-fs

Conversation

@okdistribute
Copy link
Collaborator

@okdistribute okdistribute commented Nov 16, 2016

This aims to fix #524 with EMFILE errors if ulimit is set to below 2048.

In chokidar, they use fs instead of graceful-fs. There are issues discussing this and the maintainers still use regular fs despite many folks asking them to use graceful-fs instead to avoid EMFILE errors.

The case for keeping fs instead of graceful-fs in chokidar is this:

''I don't think it will make sense to switch all of chokidar to graceful-fs because it creates a risk of locking up the program in certain watch modes that hold open too many file descriptors, as described above. In this case it seems better to let the EMFILE happen, resulting in the user educating themselves about ulimit, etc. I considered sniffing for it and printing a console.warn or something like that to tell end-users about the ulimit thing, but I don't think the maintainers of the dependents of chokidar would appreciate it polluting the stderr of their apps.''

However, we won't have that problem, because we are closing file descriptors as we go. See paulmillr/chokidar#141 (comment): 'graceful-fs will be very bad if you are opening and keeping open a lot of file descriptors'

I feel confident using graceful-fs and then closing #524

@joehand
Copy link
Collaborator

joehand commented Nov 16, 2016

Ah cool! I had looked at this but didn't realize you could do a global patch on fs. This looks good to me.

@max-mapper
Copy link
Collaborator

+1 on the rationale for using graceful-fs.

Did we determine that the isDirectory function is the source of the EMFILE errors, and that this fixes it?

@joehand
Copy link
Collaborator

joehand commented Nov 16, 2016

Did we determine that the isDirectory function is the source of the EMFILE errors, and that this fixes it?

No, it's watching too many files with chokidar. But this has a global fs patch too. I think that means it'll use that in chokidar?

gracefulFs.gracefulify(realFs)

@okdistribute
Copy link
Collaborator Author

Yes, @joehand you're right. It's monkey patching chokadir.

@beingalink
Copy link

Jfyi, I was getting this error with my test case of seeding only one big file (ubuntu install image) in a folder.

@joehand joehand merged commit cdf2be9 into master Nov 16, 2016
@joehand
Copy link
Collaborator

joehand commented Nov 16, 2016

Jfyi, I was getting this error with my test case of seeding only one big file (ubuntu install image) in a folder.

@beingalink Ya, that is another bug we know about. But EMIFILE error will be an issue regardless, may as well fix it now =).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Too many Files Being Watched (EMFILE)

5 participants