buck2 icon indicating copy to clipboard operation
buck2 copied to clipboard

project.ignore doesn't work with inotify

Open jhdub23 opened this issue 2 years ago • 14 comments

Does the buck2 daemon using inotify honor the project.ignore setting? My .buckconfig file contains:

[project]
ignore = ext

The ext directory contains 3rd party files (headers and libraries) that are used as dependencies and is very large (100k+ files). We treat these as system files and don't need to track changes to those files. Running buck2 runs into the system fs.inotify.max_user_watches limit. Output:

>buck2 targets //...
Build ID: d38e98b1-262a-45e8-ac69-1c1dcc4bb097
Error initializing DaemonStateData

Caused by:
    0: Error creating a FileWatcher for project root `<<redacted>>/dev`
    1: Creating notify file watcher
    2: OS file watch limit reached. about ["<<redacted>>/dev/ext/boost_1_71_0/src/boost_1_71_0/boost/phoenix/core/detail"]
Build ID: d38e98b1-262a-45e8-ac69-1c1dcc4bb097
Jobs completed: 0. Time elapsed: 0.0s.

Note that the error message shows that the problem is with the "ext" directory which should have been ignored. Removing the directory makes the failure go away.

jhdub23 avatar Oct 28 '23 00:10 jhdub23

Yes, it does honor that setting, I can effectively guarantee it — I have projects using both watchman and inotify and have tested the behavior quite a bit. There are some bugs around it but it does work in general (see #417 for an edge case with this setting, even if the issue isn't about that setting specifically.)

I suspect the actual behavior you're seeing here is because you have targets that take these files as input (of type attrs.source()), right? project.ignore is not meant to exclude input source files from the daemon's file watching semantics. It is meant to exclude non-source files, like build directories, where you're just incurring meaningless events when they are watched. That should probably be more explicit, frankly.

But honestly, the reality is that those files aren't actually that special. "System files that don't need to be tracked by the build graph" is explicitly a non-hermetic footgun, for reasons like this. If they are source files, and they are in the repository, they need to be tracked like everything else. inotify (and watchman) are important optimizations to make this practical exactly for repositories like yours with 100s of thousands of files, because it means the inputs needed to construct a valid build graph are proportional to the modified set of files, not the entire set of input source files.

There could probably be a "no file watching at all mode" I guess, but I suspect it would just perform worse in a lot of cases, and probably on repositories exactly like yours — a simple example might be HDDs, where you can't even use parallel threads to speed up traversing the filesystem to look for changes, due to the sequential nature of the disk making concurrent I/Os an anti-optimization.

Finally: have you tried actually increasing your inotify limits? For repositories as large as yours, I suspect that's honestly the real answer here. You can try something like:

echo 999999 | sudo tee -a /proc/sys/fs/inotify/max_user_watches
echo 999999 | sudo tee -a /proc/sys/fs/inotify/max_queued_events
echo 999999 | sudo tee -a /proc/sys/fs/inotify/max_user_instances
buck2 kill

Which might do the trick.

thoughtpolice avatar Oct 28 '23 13:10 thoughtpolice

Also, if you really want this, a different workaround might be to change references to these input files from attrs.source() to attrs.string(), or just general string concatenation, which will exclude the build system from tracking the files in this way.

You could for example just throw some flags onto the cflags parameter, i.e. add -I ext/ onto every single instance of cxx_binary that you call. However, this is explicitly non-hermetic, i.e. it will not work with remote execution, because buck2 won't know to upload files under ext/ into the remote system. This also means that by definition, your build graph is not complete, and there will be cases where changes to ext/ do not result in proper downstream recompilation. Do not do this unless you fully accept all of that and understand the consequences — because the only solution to those problems are "go back to square 1 and tracking them all as .source() files."

thoughtpolice avatar Oct 28 '23 14:10 thoughtpolice

@thoughtpolice I like the idea of 100% hermetic builds and we try to do this in our current build system. What I'm wrestling with is the practical side of implementing this at scale, especially with remote builds. We currently have many workarounds to deal with data sizes.

I did change the inotify max_user_instances system limit to handle the 100+k files. It took buck2 many hours (I let it run overnight) to catalog and do whatever it needed to with inotify. Granted, once the buck2-daemon was ready, the buck2 do-nothing build time is very fast, but we can't afford to wait hours/overnight for a 1st build. This bootstrap time before we can do anything makes the system impractical.

What can we do about the startup time to start watching 100k+ files?

jhdub23 avatar Oct 30 '23 17:10 jhdub23

Note that watchman I think will amortize those costs, but I don't know if it will actually improve startup time, so that might be worth checking out. It should at least decouple the need to have the buck daemon synchronize everything; it will instead contact watchman over a socket asynchronously. But that's just a bandaid in some sense.


However, I suspect that 100k plus files in the working set is pretty extreme Buck2 OSS use cases right now. Perhaps someone from Meta can chime in on their workflow (/cc @ndmitchell @cjhopman!); in their case though, they have buck2, watchman and their version control system (Sapling) all working in tandem to "virtualize" their extremely large monorepository. I think the idea is something like:

  1. You use Sapling to check out a "thin" virtual clone that only has a very small subset of files (using FUSE or ProjFS, or whatever)
  2. You use watchman to watch those files in your virtual working copy. Watchman also integrates with the sapling server. Watchman can poll and receive events about when files change through this channel, along with the operating system events informing it.
  3. Buck talks to watchman to get the modified files list when it needs to compute what input files to rebuild.

The scenario this is intended to handle is something like this, I think:

  1. You clone a project which only clones a small subset of all needed files. Let's refer to revisions by the order they come from source control; say this version of the repository is "version 1,000", i.e. there are 1,000 linear commits on the branch at this time.
  2. You make some changes, watchman sees them
  3. You buck2 build :a, and Buck2 is told by watchman what files changed, and it can from there calculate the build graph for target :a. Some inputs may come from a build cache; for this case, any inputs that don't exist within your clone will probably come from the cache. For example, your BUILD file may declare a dependency third-party//jemalloc, which does not change often, and your clone may not include third-party//jemalloc either. But Buck2 can just pull a copy of it from the cache, instead, which is fine.
  4. You rebase onto a more recent HEAD, say, 100 commits (so the repository is at revision 1,100), which may include thousands of files of changes. Let's say one of these made a change to jemalloc to upgrade to a new version at revision number 1,075.
  5. Because you rebased, watchman is informed of all the changed files from those intermediate 100 commits, even though they may not exist in your small virtual clone. This includes jemalloc, which is an input to your program, but does not exist in your repository.
  6. You run buck2 build again. Buck2 then is given the input set of changed files to construct the new build graph, which includes "rebuild jemalloc"

Step 5 is very important: an invariant of source control in this model is that after revision number 1,075 then all future revisions should use that version of jemalloc. So if watchman does not talk to Sapling in Step 5, this does not happen, and step 6 uses an old input. This is a classic case where the build system does not properly rebuild after an input changed, i.e. an untracked dependency. In short, you must query the source control system for the build graph to remain fully complete.

So I bet this kind of setup would work really well for you in this case. Not to mention it would avoid all the filesystem overhead of tracking those extra 100,000 files when you practically will only need a subset of them many times. Sapling is open source, but the virtualized server is not yet ready for OSS use. So, I don't think there's an equivalent source control virtualization solution for a scenario like the one above...

I'm not sure what the direct answer here is, immediately. I think some more specifics might be needed in this case if you can provide them, but Neil or Chris probably have better ideas here...

thoughtpolice avatar Oct 30 '23 18:10 thoughtpolice

I would love to have the entire buck2/sapling/watchman/virtual-file-system ecosystem but was hoping we could just start with buck2 and maybe watchman. Basically, replace make/cmake/scons/etc. with buck2. Is there some workaround for the startup cost? You are correct, @thoughtpolice. Amortization with watchman wouldn't help if the time to first build remains hours/overnight.

Our workaround in our current build system is to ignore major portions of the dependencies in ext. Obviously, this is not great, but ext is essentially treated as a read-only file system and we encode the entire directory for build purposes. For example, ext/version-<version_hash>. The files are not allowed to change; if we need updates, we release a new version-<version_hash> for consumption. This path is embedded as a dependency for binary caching purposes. All binaries are directly dependent on the version-<version_hash> and not the contents of 100+k individual files.

jhdub23 avatar Oct 30 '23 18:10 jhdub23

More specifically, we are building C++ using scons. Scons hashes the command line as a direct dependency.

gcc ... -isystemext/version-hash1 ...

The files in ext/version-hash1 are not tracked for changes (scons ignores tracking -isystem files. However, if we change the dependency path:

gcc ... -systemext/version-hash2 ...

a new build would be triggered as the command line itself has changed.

jhdub23 avatar Oct 30 '23 19:10 jhdub23

Interesting note on the ext versioning and the path changing. I think that would be OK if you used hash_all_commands = true in order to cause appropriate rebuilds.

I wonder if it might be possible to take ext/version-123 and put it in a tarball named ext-123.tar.gz, and then download that tarball with http_archive? This will avoid rewatching the files for inputs, since they'll be "outputs" under buck-out. You could then attach that directory to your prelude//toolchains:cxx through various arguments... Something like this (probably broken)

EXT_VERSION = "123" # arbitrary

http_archive(
    name = 'cxx-toolchain-ext'
    urls = 'https://mycorp.example.com/downloads/ext-{}.tar.gz'.format(EXT_VERSION),
    sha256 = ...,
)

system_cxx_toolchain(
    name = 'cxx',
    c_flags = [ "-I $(location :cxx-toolchain-ext)/include" ],
    cxx_flags = [ "-I $(location :cxx-toolchain-ext)/include" ],
    ld_flags = [ "-L $(location :cxx-toolchain-ext)/lib" ],
)

That is, Buck will first download the tarball, and then set up your C++ toolchain to always have those -I flags passed, pointing to its location. The $(location) macro does exactly that. Assuming that this was the definition of your prelude//toolchains:cxx target, it might work? Two downsides:

  • You now have to update the ext.tar.gz file out of band, which sucks, and might be painful. It probably has to be done by CI.
    • But maybe you could keep ext/ it in the repository and automate that task in the same place? You just can't put it inside the build graph is all...
  • Extracting the tarball might be slow.

If you update the ext/ package every day or multiple times a day, this might suck. But if you only do it rarely (maybe once a quarter, few weeks, whatever), it's probably not so bad?

This will avoid the file watching overhead but does make the design a little clunkier. However, it might avoid the absolutely massive performance penalties you see. Maybe worth giving it a shot? You should be able to get an experiment running pretty quickly if you just tar up the directory yourself and serve it over localhost. A modified BUILD file like the above can then be whipped up quickly.

thoughtpolice avatar Oct 30 '23 19:10 thoughtpolice

Also just to be clear, there may simply be a major set of bugs and optimizations with the existing inotify support that cause this! Maybe we're just holding it wrong and it can be made 50000x times faster. I definitely wouldn't rule that out. This would need to be reproduced more throughly, I'm just trying to help unblock you I hope. :)

thoughtpolice avatar Oct 30 '23 19:10 thoughtpolice

Thanks, @thoughtpolice. Our ext is under strict control and does not change that often. I'll try experimenting with the ext-123.tar.gz and http_archive. In concept, it's similar to how we are currently managing ext dependencies with the path:

Concept Current build system Buck2 build system
Root dependency ext/version-xyz (path only) ext-xyz.tar.gz
Dependent files shared file system, pre-populated buck2 must extract the tarball
Compilation control -isystemext/version-xyz/include -I $(location :cxx-toolchain-ext)/include

The downside of buck2 is the extra disk space and performance. Every user must unpack the (very large) ext-xyz.tar.gz, while in our current build system, it's just a symbolic link to a shared file location. In our build system, only users who are working on ext need to make a local copy for subsequent modification (and very carefully control of the ext dependency through path manipulation).

I guess you take care of this through your virtual file system, which is what we are "faking" with our ext.

jhdub23 avatar Oct 30 '23 22:10 jhdub23

Yes, and if the file is just too unreasonably large to download or whatever (especially if you accidentally deleted it), I think you could just get away with doing what you're doing now — using -isystem ext/version-xyz/include in your prelude//toolchains:cxx definition, and keep ext/ as a symlink to some shared filesystem out of the repository. Note that buck always executes all commands from the root of the project (i.e. where .buckroot is located) which is why that would work; so this will not be hermetic according to Buck, but it would be the "moral equivalent" of what you're doing now, anyway.

thoughtpolice avatar Oct 31 '23 00:10 thoughtpolice

I'm beginning to suspect that there is indeed a bug with projects.ignore and inotify.

Using buck2 getting started, after cloning and building with buck2, I used inotify-consumers to monitor the watches. The following trace shows that the ".git" directory is being watched, even though it should be ignored. Notice that the watch counts match the find command counts as the ".git" directory is added/removed.

After the 1st build:

>./watches.bash
   INOTIFY   INSTANCES
   WATCHES      PER   
    COUNT     PROCESS   PID USER         COMMAND
------------------------------------------------------------
     129         1     <redacted> <redacted>      buck2d[hello_world] --isolation-dir <redacted>

>find . -type d -print | wc -l
129
>tail -2 .buckconfig 
[project]
ignore = .git
>cp -R ../../.git .
>find . -type d -print | wc -l
146
>./watches.bash 
   INOTIFY   INSTANCES
   WATCHES      PER   
    COUNT     PROCESS   PID USER         COMMAND
------------------------------------------------------------
     146         1     <redacted> <redacted>      buck2d[hello_world] --isolation-dir <redacted>
>rm -rf .git
>./watches.bash 

   INOTIFY   INSTANCES
   WATCHES      PER   
    COUNT     PROCESS   PID USER         COMMAND
------------------------------------------------------------
     129         1     <redacted> <redacted>      buck2d[hello_world] --isolation-dir<redacted>

jhdub23 avatar Nov 02 '23 23:11 jhdub23

The project.ignore currently gets applied after watching. So we watch everything, then if we see a change trigger that is on the ignore, we drop it then. That's obviously a problem for you! As to solutions:

  • We could incorporate the ignore list into the watcher. For Watchman this doesn't make any sense (it watches everything anyway), but for the notify path it probably does. We currently use the Rust notify library which lets us watch a path recursively, or not watch it - https://docs.rs/notify/6.1.1/notify/trait.Watcher.html. To incorporate fine grained ignores we'd probably have to watch the root, then add/remove watches recursively ourself. That would be a lot of work. Ideally we'd push that into the notify library (e.g. take a list of directories to ignore). Not too hard to program, but a moderate amount of work.
  • You could switch to Watchman. With Sapling/Eden etc that would give a lot of benefits, but just Watchman alone means you can use the .watchmanconfig to entirely ignore directories - https://facebook.github.io/watchman/docs/config#ignore_dirs. Even if it uses inotify, that should create a lot less notifiers.
  • You could restructure your repo so ext was outside the Buck2 bit. You can do that with an http_archive or by having ext and src as top-level directories, putting all your source code in src, having that be the root of the Buck2 project, and then using symlinks outside the project to ext to get your third-party stuff. At Meta we kind of do that approach, but the ext is on a shared drive - and the path has the version information/hash embedded in it.

ndmitchell avatar Nov 22 '23 16:11 ndmitchell

@ndmitchell Thanks for confirming the behavior of Buck2 with inotify. We are experimenting with both Watchman and the htpp_archive solutions, plus a third solution: dynamically creating a link to ext as a genrule().

jhdub23 avatar Nov 28 '23 00:11 jhdub23

Also to note, I was seeing a problem with changes to BUCK files or .buckconfig not taken into account which I assume could be related to a problem running out of inotify watches. I'm still investigating, but the problem manifested itself with a running buck2d when adding / deleting targets but buck2 uquery ... would not list added, still show deleted targets (although the BUCK file was indeed reported as having changed) and config changes would just have no effect.

avdv avatar Jan 15 '24 09:01 avdv