-
Notifications
You must be signed in to change notification settings - Fork 48
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
Use fanotify on Linux? #87
Comments
Looks nice. I believe when I started this project it wasn't widely available yet. If we can detect support for it at runtime, I think it's worth having with a fallback to inotify. I imagine some distros don't have it yet. |
I think we'd have to determine the kernel version itself:
The two (Ubuntu 18.04 LTS uses the 4.15 kernel and is supported until April 2023. All newer versions are on 5.1+.) |
There are a number of details in the |
I believe there are a few possible ways we could call
All have better support for renames than inotify and higher default limits than inotify, so even the first bullet has advantages when compared to the existing inotify watcher. Moving this discussion over from fsnotify/fsnotify#114, @amir73il is my summary above correct? Maybe the nicest thing to do would be to implement these as different backends such as Also, it would be nice to have an option of a What do you think @devongovett ? |
unfortunately, FAN_MARK_MOUNT does not support watching the events that you are interested in (create, delete, rename) I recommend to watch FAN_RENAME event and not FAN_MOVED_TO/FROM because it is not easy to pair them. |
Thanks as always for the feedback @amir73il! If we're able to implement @devongovett if you're okay with the plan I laid out above, I will I'll find someone to work on this and sponsor them to send a PR since I know Node, but not C++. And to be clear about my motivations, if I can implement |
I haven't had a chance to look into fanotify much yet, but seems reasonable to me. Based on your summary above, it seems like I guess the |
Yes, that's my understanding
Yeah, the two main ones would be the higher default limits (~4x higher on my machine as shown below) and support for reporting file moves.
I had originally thought you'd only need the special permission if you're running inside Docker, which wouldn't be as big of a limitation, but I just looked a bit more and think I may have been mistaken on that. So yeah, it might not be worth implementing. In either case, it's not what I would be interested in implementing because Immich is nearly always deployed within Docker and so I know I can't use it in either case. |
This raises another point: currently renames are reported by |
My thought was to make it an option. Continue to report a delete followed by a create by default for cross-platform compatibility, but enable users to opt-in to receive only a single move event on Linux. Many applications are known to run only on Linux because a company runs them on their servers or cloud or because they're deployed in a container. This is a critical functionality for some of those applications (such as Immich). This can't easily be added to most other Linux watching libraries, since it requires a native component to utilize You can report file moves on MacOS and Windows by comparing the inode ID of files in create and delete events, but this will only allow you to match up create and delete events within some specified timeout window. |
Making it opt-in sounds reasonable to me. 👍 |
I'd like to take a jab at this (saw the listing from @benmccann), could someone tell me what are the C++ standards available to be used here (or the toolchains you use) and any tips on locally testing the changes. (The documentation leaves something to be desired 😅) |
@devongovett any chance you might be able to take a look at the PR that was sent for this? |
Curious if y'all have thought about using https://man7.org/linux/man-pages/man7/fanotify.7.html to watch on Linux? It's much more performant than inotify. You can monitor all changes on a fs mount — which I assume that + filtering out any changes not related to the directory of interest would be pretty performant.
The text was updated successfully, but these errors were encountered: