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

fix: update fsevents via chokidar to fix MacOS bug #17938

Closed
wants to merge 15 commits into from

Conversation

Js-Brecht
Copy link
Contributor

@Js-Brecht Js-Brecht commented Sep 27, 2019

Description

This is a rather quirky issue, where gatsby develop (and likely gatsby build) will hang at various stages while using MacOS. Most common was source and transform, and usually when running plugin gatsby-source-filesystem. It also hangs in dev-404-page, internal-data-bridge, and others.

As it turns out, there is an issue with chokidar.watch() on MacOS. It uses fsevents by default, in an effort to increase speed and reduce cpu load.

Disabling fsevents has been tested by @ehowey, who was experiencing this issue regularly. It has been shown to result in a successful build. It will also increase the load on the cpu, so for larger sites, there will be a performance impact. Use env var CHOKIDAR_USEPOLLING=1 to disable fsevents.

UPDATE 10/10/19
The issue has been tracked down to a bug in fsevents itself. Seems there was a race condition that was causing the thread to hang. Thanks to @RomanHotsiy for his hard work! 🎉 🎉 As soon as the fix has trickled down, we can do updates to fix the problem.

See comment here for instructions for testing

UPDATE 10/13/19
fsevents@2.1.1 has been published, which resolves the issue. Just requires updating lockfiles.

Related Issues

Fixes #17131

@Js-Brecht Js-Brecht added type: bug An issue or pull request relating to a bug in Gatsby topic: cli Related to the Gatsby CLI topic: plugins labels Sep 27, 2019
@Js-Brecht Js-Brecht requested a review from a team as a code owner September 27, 2019 06:14
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holy buckets, thanks for the clear explanation. Could you add comments to the useFsEvents property?

Awesome 🕵! Thanks for creating this great workaround.

packages/gatsby/src/utils/get-static-dir.js Outdated Show resolved Hide resolved
packages/gatsby/src/query/query-watcher.js Outdated Show resolved Hide resolved
packages/gatsby/src/commands/develop.js Outdated Show resolved Hide resolved
packages/gatsby-source-filesystem/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-page-utils/src/watch-directory.js Outdated Show resolved Hide resolved
packages/gatsby-dev-cli/src/watch.js Outdated Show resolved Hide resolved
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holy buckets, thanks for the clear explanation. Could you add comments to the useFsEvents property?

Awesome 🕵! Thanks for creating this great workaround.

@wardpeet wardpeet changed the title Fix issue with chokidar.watch() causing hang during build in MacOS when using fsevents fix: disable fsevents on chokidar to fix MacOS bug Sep 27, 2019
@KyleAMathews
Copy link
Contributor

How does this PR change performance on a larger site?

Js-Brecht and others added 2 commits September 27, 2019 06:04
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
@Js-Brecht
Copy link
Contributor Author

@KyleAMathews I'm very interested in the answer to that question as well. Theoretically, it shouldn't hurt any. Basically, what this does is fallback to polling kqueue, and because of the nature of libuv, polling takes a back seat if there are any active setTimeout(), nextTick(), Promises, handlers/setImmediate(); so, it shouldn't be blocking processor time for any running processes.

I think it would have to do with how chokidar implements the polling mechanism. TBH, I don't know a lot about chokidar; I've never looked at their code. I'd very much like to see this tested on a variety of different sites, small & large, especially those that extend the build process beyond the standard.

Js-Brecht and others added 6 commits September 27, 2019 06:07
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
Co-Authored-By: Ward Peeters <ward@coding-tech.com>
@Js-Brecht
Copy link
Contributor Author

@wardpeet Done. Sorry, didn't even think about that. I was just focused on getting it out, so I could go to sleep 😄

@ehowey
Copy link
Contributor

ehowey commented Sep 28, 2019

@Js-Brecht Wow - you worked fast on that! I tested it on my end - it seems to completely fix this error for me. I tested it with both gatsby-starter-hello-world and gatsby-advanced-starter. In both cases I ran gatsby-dev --scan-once followed by the gatsby-develop command 10+ times in each repo. I can report that it did not hang or stop in the build process ever.

@Js-Brecht
Copy link
Contributor Author

@ehowey Do you have any production sites, or sites you're developing that might be more complex that you could test it on?

@Js-Brecht
Copy link
Contributor Author

Need to stress test it

@ehowey
Copy link
Contributor

ehowey commented Sep 30, 2019

To be honest, not really. My "production" sites are pretty bare bones when it comes to the size and complexity of the site. Not in a way that provide any kind of system stress anyways. Maybe I could pull down a larger open source site and do something with that? Got any ideas that might fit the bill?

@paulmillr
Copy link

Chokidar maintainer here. The performance would definitely be shitty after this change. Be careful.

fsevents uses one handler for 100,000 files. When it's disabled, chokidar would use 100,000 handlers. Which will use tons of RAM and CPU.

@Js-Brecht
Copy link
Contributor Author

@paulmillr good to know, thanks. Perhaps you could weigh in on the cause of the issue, and if there's a potential fix? TBH, I think it would be a better solution for the root of the issue to be addressed than to actually use a band-aid fix like this one.

@Js-Brecht
Copy link
Contributor Author

Js-Brecht commented Sep 30, 2019

Nevermind, I see you have a fix in place already. Excellent!

@paulmillr
Copy link

Yeah, try chokidar@master — if it doesn't work, please provide more detatils. I've ran your loop and it didn't end, everything was fine. Perhaps it's related to "put disks to sleep" setting on MacOS or something like that.

@Js-Brecht
Copy link
Contributor Author

Hey guys, sorry it's taking me so long. I've been working overtime on a critical business process @ work.

I've updated the PR so that you can opt out of using fsevents, since this still seems to be an issue with the updated version of chokidar. This way, it won't effect everyone, but if anybody is experiencing this issue and they aren't concerned about performance (small site?), they can set GATSBY_USE_FSEVENTS=0, and it will disable it.

I have a little time today, so I'm going to begin work on the reproduction repository, so we can start moving towards an actual resolution.

@paulmillr
Copy link

paulmillr commented Oct 6, 2019

There is no need for the env var. use CHOKIDAR_USEPOLLING.

@Jeddf
Copy link
Contributor

Jeddf commented Oct 6, 2019

Sounds like there is high confidence already but I did some runs on my mac to sanity check the disable fsevents workaround flag. No obvious problems with functionality either:

15/15 runs not hanging with GATSBY_USE_FSEVENTS=0 gatsby develop

@Js-Brecht
Copy link
Contributor Author

Even better, @paulmillr; glad you're here, because I completely missed that in the docs. Definitely no reason for our own env var, if there is one that chokidar itself uses to do the same thing. Will save in future maintenance, for sure; less to remember to do if another instance of chokidar.watch() needs to be used anywhere.

I'll update this PR later today, and just leave it as an upgrade. It works out, because that's likely what it will be anyway, assuming I can narrow down the issue.

@muescha
Copy link
Contributor

muescha commented Oct 7, 2019

maybe it would be nice have some docs about this and what this env var solves?

@Js-Brecht
Copy link
Contributor Author

@muescha I will be removing it. I didn't realize there was an env var for chokidar already existing, so this would just be redundant. Instead of GATSBY_USE_FSEVENTS=0, people will be able to use CHOKIDAR_USEPOLLING=1.

@muescha
Copy link
Contributor

muescha commented Oct 8, 2019

i mean documenting this CHOKIDAR_USEPOLLING and to show what problem it should solves in combination with gatsby. if you have this problem with fsevents then maybe other users have the same problem.

maybe in some new troubleshooting doc?

@wardpeet
Copy link
Contributor

wardpeet commented Oct 8, 2019

@Js-Brecht We could just set the polling to true at the start of gatsby-cli. If we make it opt-in, we probably want to put it in https://www.gatsbyjs.org/docs/debugging/ or something.

@Js-Brecht
Copy link
Contributor Author

Sure, I can write up a doc for it.

I'm hesitant now to make it the default setting, @wardpeet, considering how it could affect some sites. It just feels to me like the most effective/efficient method should be the default, and while there are more than just a couple people experiencing this, it's not really enough to warrant bombing everybody.

v3.2.1 seemed to help, some; reduced the frequency, anyway. I think I might be making some headway on a reproduction, too.

@RomanHotsiy
Copy link
Contributor

RomanHotsiy commented Oct 10, 2019

Looks like this is caused by fsevents issue. I already opened a PR to fix it: fsevents/fsevents#290

For me it is fixing the issue completely.

Would be great if someone else can try it also.

Here is how to test it:

  • clone the forked fsevents repo: git clone --single-branch -b fix/pthread_cond_wait-race-condition https://github.com/Redocly/fsevents.git
  • run npm run build to produce fsevents.node
  • replace node_modules/fsevents/fsevents.node in your gatsby site dependencies with the resulting fsevents.node from the previous step
  • run gatsby develop or npm start in your gatsby site

cc @Js-Brecht @paulmillr

@ehowey
Copy link
Contributor

ehowey commented Oct 10, 2019

@RomanHotsiy I got the following error trying access your repo:

git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Do you need to update the permissions?...EDIT: NVM, just had to change the clone location and it worked, https://github.com/Redocly/fsevents.git

@Jeddf
Copy link
Contributor

Jeddf commented Oct 10, 2019

Looks to have completely solved it for me too 🎉 Awesome!

It didn't work at first but then I realised I had a few nested node_modules/fsevents to copy my fsevents.node file to. This find command that google turned up was useful for listing them all: find . -path '*/fsevents.node'

@Js-Brecht Js-Brecht added type: upstream Issues outside of Gatsby's control, caused by dependencies and removed topic: cli Related to the Gatsby CLI topic: plugins labels Oct 11, 2019
@ehowey
Copy link
Contributor

ehowey commented Oct 11, 2019

I tested it on the hello-world starter and also on a more complex project I am working on using themes and workspaces. It seems to fix it entirely! You do have to make sure you track down all instances of fsevents.node as I had a few of them tucked into the node_modules folder in a couple of spots as commented on above.

@Js-Brecht and @RomanHotsiy it was awesome collaborating with you both to track this bug down and I learned some stuff along the way. Kudos to your persistance in figuring this one out. Way beyond what I ever would have gotten to on my own.

@Js-Brecht
Copy link
Contributor Author

Js-Brecht commented Oct 11, 2019

Can you guys drop a list of the packages that are using fsevents? I just want to look at their fsevents dependency version specifier.

FYI, you can use yarn resolutions if you want to override the version of fsevents being used down through your dependency tree. Or, if you're using pnpm, check out pnpm hooks (pnpm is pretty cool). Using either one of those, you can point the dependency for fsevents to @RomanHotsiy's fork.

Or, I guess, point it to where you built it on your own disk, if he doesn't have it built in his repo.

@Jeddf
Copy link
Contributor

Jeddf commented Oct 12, 2019

chokidar, gatsby, gatsby-page-utils, gatsby-source-filesystem

yarn list v1.19.0
├─ fsevents@1.2.9
├─ gatsby-page-utils@0.0.25
│  └─ fsevents@2.1.0
├─ gatsby-source-filesystem@2.1.31
│  └─ fsevents@2.1.0
└─ gatsby@2.15.36
   └─ fsevents@2.1.0

thanks for the yarn resolutions tip

@Js-Brecht
Copy link
Contributor Author

Oh, I was thinking they were external dependencies 😆

@paulmillr
Copy link

New fsevents is out, so just rebuild your lockfiles - chokidar will auto-use it

@Js-Brecht
Copy link
Contributor Author

Js-Brecht commented Oct 14, 2019

Thanks @paulmillr.

@wardpeet not really a need for this PR anymore issue is resolved by fsevents@2.1.1; PR updates lockfiles/dependencies, but not really needed except maybe for Gatsby developers. Unless you guys still want me to write a doc for the CHOKIDAR_USEPOLLING switch for debugging, we can close/merge it.

@Js-Brecht Js-Brecht changed the title fix: disable fsevents on chokidar to fix MacOS bug fix: update fsevents via chokidar to fix MacOS bug Oct 14, 2019
@wardpeet
Copy link
Contributor

@Js-Brecht if it's resolved we don't really need to write a doc about it. Thanks for your work and let's close this for now.

Thanks everyone for jumping on this thread! <3

@wardpeet wardpeet closed this Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby type: upstream Issues outside of Gatsby's control, caused by dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fsevents bug] Stuck at "source and transform nodes" / "createPagesStatefully" on MacOS
8 participants