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

Polling fallback #9

Open
nathany opened this issue Jun 29, 2014 · 38 comments
Open

Polling fallback #9

nathany opened this issue Jun 29, 2014 · 38 comments
Labels

Comments

@nathany
Copy link
Contributor

nathany commented Jun 29, 2014

Whether or not fsnotify implements polling support itself, some thought should be given into how polling could work as an alternative to native OS events.

GoConvey uses polling exclusively to avoid the "too many files" error #8.

"we walk the file system every quarter second and use the sum of the last mod time stamp and size of each go file as a quick comparison. Along the way we make note of new and deleted packages, all the while skipping 'ignored' packages. It's actually quite speedy." - @mdwhatcott, Gopher Slack

If you use an approach like what I've done in GoConvey, make sure to count empty directories as well... - @mdwhatcott

https://github.com/smartystreets/goconvey/blob/master/web/server/watcher/scanner.go

@pifantastic reports that node's gaze (https://github.com/shama/gaze#errors) library detects EMFILE errors and falls back to polling.

Polling could be opt-in for network file systems (NFS), Vagrant or Plan 9 -- where OS events don't work or are unavailable.

@mdwhatcott
Copy link

FYI, the link (above) to the goconvey scanner is no longer valid (I've recently rewritten that package. My approach, however, is unchanged:

https://github.com/smartystreets/goconvey/tree/master/web/server/watch

@omeid
Copy link

omeid commented Jan 31, 2015

👍 for polling.

It is not ideal, but it is better than none and instead of everyone developing it on their own, it makes sense to have one that is made better by many people.

In terms of the opt-in option, fsnotify could have an option to "register" different "watch providers", in the spirit of sql package. This would also allow people to develop their own providers for other services, say Dropbox, s3, and so forth, but maybe that is jumping ahead of ourselves.

@nathany
Copy link
Contributor Author

nathany commented Jan 31, 2015

I've never even thought of extending it to webhook notifications like Dropbox and Google Drive provide. Interesting idea. Not sure if it belongs in this package, but having a common interface in top could be cool.

The sql driver model is one I've been thinking about, particularly if there are multiple options for a given OS.

@omeid
Copy link

omeid commented Jan 31, 2015

Yeah, it really depends how you look at it, I wouldn't consider them as general webhooks as you can think of dropbox or s3 just another type of filesystem.

Regardless of that, having a watch-driver interface and different driver is a better design IMHO.

@nathany
Copy link
Contributor Author

nathany commented Jan 31, 2015

Indeed. It also fits with another desire I have, which is to make it easier to contribute to without being knowledgable of every single platform. Separate drivers for inotify, kqueue, etc. would be one way to achieve that.

@szechyjs
Copy link

👍 polling would be great. I'm trying to migrate a python application from watchdog to fsnotify. Unfortunately I think this is the roadblock, as the filesystem that needs monitored is on NFS and polling is the only option.

@nathany
Copy link
Contributor Author

nathany commented Jun 26, 2015

Yah, polling is the only option for NFS as far as I know. You can also check https://github.com/rjeczalik/notify but I don't think it has polling yet either.

@calavera
Copy link

You might be interested in checking how docker support fsnotify and polling at the same time. We have a compatible interface and a fallback initialization for when fsnotify is not supported:

https://github.com/docker/docker/blob/master/pkg/filenotify/filenotify.go

maybe @cpuguy83 would be interested in moving the polling here so we can maintain only one package.

@cpuguy83
Copy link
Contributor

That would be great.
The reason I did not submit here is it seemed rather unknown if polling support was desired and we really needed to implement it.

@nathany
Copy link
Contributor Author

nathany commented Dec 11, 2015

Please do submit a pull request.

@omeid
Copy link

omeid commented Dec 13, 2015

@nathany Any plans for the v2 with driver interfaces? I will be happy to help out.

@nathany
Copy link
Contributor Author

nathany commented Dec 14, 2015

@omeid I don't have any plans for what a driver interface would look like yet (and to be honest, I haven't done much work on fsnotify lately).

Personally, I'd prefer to see the current code base cleaned up before doing a big API change. The Windows internals are particularity crufty.
https://github.com/go-fsnotify/fsnotify/milestones/v2%20Internals

Maybe we could start a new issue to figure out the details of transitioning to a driver model?

@cpuguy83
Copy link
Contributor

@nathany Can you clarify what you'd like to see?

@nathany
Copy link
Contributor Author

nathany commented Dec 16, 2015

@cpuguy83 Do you think you would be able to add polling without changing the API? Maybe just for operating systems that fsnotify doesn't currently support?

There are other situations where polling would be desirable, but I'm not sure how to detect them, or if it should be done more manually (which is why a driver-style API is relevant to this discussion).

@nathany
Copy link
Contributor Author

nathany commented Dec 16, 2015

In terms of API, here are related issues #104 and #75.

Polling should also help with #45 for Windows users.

@nathany nathany added this to the v2 Internals milestone Dec 16, 2015
@nathany
Copy link
Contributor Author

nathany commented Oct 2, 2016

@mdwhatcott @cpuguy83 Would you be willing to build a stand-alone fsnotify/polling package that could be incorporated into fsnotify as a secondary step?

If so, I'll create a repo called polling or poller or whatever you prefer.

For me the key considerations are:

  • providing a speedy implementation as outlined in the original post
  • a simple and idiomatic Go API specifically for polling
  • solid unit and/or integration tests (preferably without any external testing dependencies to keep it simple)
  • well documented both in the README, examples, and API docs

@mdwhatcott
Copy link

@nathany - Sounds like a fun project but I can't commit to it at this time.

@nathany
Copy link
Contributor Author

nathany commented Oct 3, 2016

ok.

Here is a polling watcher by @radovskyb https://github.com/radovskyb/watcher

@nathany
Copy link
Contributor Author

nathany commented Oct 18, 2016

@radovskyb Hey Benjamin, Would you be interested in transferring watcher into the fsnotify organization and working on it here? Still as a stand-alone repository for the time being.

The API already looks pretty close to fsnotify.

Once some of the low-level bits are extracted from fsnotify (e.g. #173) I'd like to incorporate polling into fsnotify as a fallback, while still allowing people to use the poller directly if that's all they want.

@Helflym
Copy link

Helflym commented Jan 29, 2020

I need this package in order to port auditbeat software (https://github.com/elastic/beats) on AIX.
However, AIX doesn't provide an easier way to watch files than polling them.
I'm planning to take some part of @radovskyb's work. But we don't have the end of your slack conversation here, @nathany and @radovskyb. I would like to know if any of you is against such thing ?

@serdarkalayci
Copy link

Just wanted to add my two cents about polling mechanism. In Kubernetes, when a ConfigMap which is attached as a file on a pod changes, the file inside the pod changes as well but its last modified timestamp does not change. Size may not change depending on the change. So the GoConvey's polling approach will not work on this scenario.
Francisco Beltrao from Microsoft offers a different approach which checks the hash of files instead of timesptamp. Here's a .NET Core implementation:
https://github.com/fbeltrao/ConfigMapFileProvider
It'll be slower than the aforementioned approach but it covers more ground.

@nathany
Copy link
Contributor Author

nathany commented Jul 31, 2021

@bep Recently added a polling fallback in Hugo that may be worth taking a look at.
gohugoio/hugo#8723

@Howie59

This comment was marked as off-topic.

@keithknott26

This comment was marked as off-topic.

@berlinguyinca

This comment was marked as off-topic.

@aashakabra

This comment was marked as off-topic.

@andreypostal
Copy link

Started working on some implementation for this issue, initial thoughts can be seen here. It only implements the recursive version for now and has a few bugs related to watching new added items (and/or improvements).

It uses multi pool workers to watch a batch of items (folders and files) in some interval - which comes from a random range. It also uses bloom filters to detect new items.

@arp242
Copy link
Member

arp242 commented May 19, 2024

Re: your email from yesterday:

I saw that there is some long discussion around this and even a pending PR, and that's why I'm sending you this email, is something like this of your interest? Are there any plans related to this or is something that is discarded already?

It's just that no one wrote any code for it. Or when they did, the code wasn't really merge-able for one reason or the other. For example that existing PR only has recursion.

The minimum requirements for it to get merged is:

  • Needs to match behaviour of existing tests, unless there's a good reason it can't.

  • Also needs to pass the optional tests; there are enabled with supportsRecurse(), supportsFilter(), and supportsRename(), and supportsNofollow() in helpers_test.go – note they're not publicly available yet, because it's not implemented in all the backends (this actually isn't so easy to do for the poll watched, see below).

  • Also looking at your PR, I'd rather not add new dependencies unless there's a very very good reason for doing so.

    Does using bloom filters really matter in real-world performance here? Maybe for some (very) large directories, but even if it did, this seems like the sort of implementation detail better left for the future And many directories are small – is it still faster then?

    Similarly, it's not clear to me that ants.MultiPool really adds too much value over just "go scanDir(...)" with some basic logic to spawn only n goroutines? Or launching n goroutines that all read from the same channel for dirs to scan?

  • Should stick to Go 1.17 and x/sys 0.13 (last version to support 1.17) unless there's a specific good reason to use something newer. I'm not against updating that if it has a concrete benefit, but preferably not beyond 1.19, since that's what the latest Debian has. Certainly not to Go 1.22.

Other than that I haven't looked too closely at your PR; there are just some high-level comments.


Actually running the tests for the poll watcher is not so straight-forward, because everything is assumed to be GOOS-based. I did a bit of work to start moving away from that a few weeks ago by splitting the Watcher type from the backends, but there's still quite a bit of work to do especially for tests. Should probably make a "runAllTests(b backend)" or something, but this requires some refactoring and such.

The supports...() for running optional tests should probably also be moved to the backend. All of this is supposed to be temporary until it's implemented everywhere.

All of that should be a separate PR. I don't mind working on that, but I don't know when. I'm mostly focused on getting the optional features in all backends.


Another general issue is what the API should look like; I spent a bit of time on it a few weeks ago; but I'm not really sure about this yet. At the very least I would like to have options to 1) set the poll interval, 2) check file contents based on hash, and 3) maximum number of parallel goroutines to scan with. Those last two don't need to be in the first version, but do need to think about the API.

@andreypostal
Copy link

andreypostal commented May 19, 2024

Hey, the case that i'm working with aims for very very large directories and also lots of directories/subdirectories. I did not perform any considerable test so far, but for development i'm using ~6k folders and subfolders with a total of 50k files (~10 level deep), which is performing well.

One of the things that i'm adding is the option to select the interval and the change based on file hash. Actually, my only idea for supporting the rename so far is using the file hash - detecting a new file that has same hash as a deleted file (let me know if you have some thoughts).

The bloomfilter I believe is a perfect fit here for tracking new files, I don't see any reason to avoid it... it would only be using more memory to achieve the same end result.

Related to ants pool, probably is something that can be replaced by an internal pool implementation, but in a scenario with lots of tasks it is better to work with it (at least I think), considering that we can benefit from multi pools, task queues, cached workers as well as cleanup. My idea on the use of ants is achieve better performance and memory usage.

For my solution I would say that both this dependencies are essential, ants and bloomfilter (I have to remove the one for maxproc).

I also want to add injection of a Logger, I'm missing it in the current usage.

Another consideration for it to be considered a valid implementation here, would be to change the way that the directories are scanned, using it with os.Open means that the physical directory gets locked, which I'm able to deal well in my company scenario because of internal lock system, but here would be problematic. But, overall I think this could bring good insights and valuable discussion (for me is important because I need to read the directory as a buffered input in batches).

@arp242
Copy link
Member

arp242 commented May 19, 2024

This is a generic project used by lots of people in all sorts of cases, and your use case is just one, and the polling backend is just one backend. Adding a bunch of dependencies here means adding a bunch of dependencies to 250,000 projects.

If we want to optimize things with a bloom filter then we can always do that later. And that optimisation also applies to kqueue, and intotify, and everything else. And maybe a tree is an even better optimisation.

But realistically, I would expect it doesn't really matter too much in the context of file I/O, which is much slower. This is like optimising the string concatenation of your SQL queries when those queries take a second to run.

I also want to add injection of a Logger, I'm missing it in the current usage.

This should be a different PR/issue.

@andreypostal
Copy link

andreypostal commented May 19, 2024

I mean... I know that, that's why it is in my repository only right? Was only meaning to help, but well... thanks for the project, good luck :)

@arp242
Copy link
Member

arp242 commented May 30, 2024

I mean... I know that, that's why it is in my repository only right? Was only meaning to help, but well... thanks for the project, good luck :)

Well you did ask if a patch could be accepted.

Anyway, I took a closer look at this today; all operations below done on a collection of 50k items, collected from my system's /usr (to give reasonable realistic strings):

  • Map lookups are always faster; but we're talking about ~12ns vs. ~50ns here, so it's not that much and basically insignificant. Map appends are also faster: ~20ns vs ~100ns. Also insignificant.

  • Bloom filter uses quite a lot less memory; for 50k entries it's 58K vs. 3,469K for a map. However, 3.5M is not that much in almost all type of cases where you're watching 50k paths (and I'm not 100% sure if that 58K number is accurate, as it seems a bit too low).

  • Deletes are very slow because it constructs an entire new bloom filter, about 6.5ms (vs. 12ns for a map). Even a slice with a for range loop is faster(!)

Performance wise the end result is that it's more or less even except deletes are much slower. Unacceptably slow in fact (more on that later). It might save a bit of memory, but I doubt anyone will notice, and I say "might" because you will need to keep a separate list of files regardless for WatchList() and perhaps other things, which you probably won't need to do for a map (which you can re-use).

Another downside is that bloom filters have the chance of false positives. If I test all 121,693 paths entries in my /usr against that list of 50k a map will give exactly 50k matches, as expected, but the bloom filter gives me 50,469 matches. This means that in some cases it may report events for paths that didn't actually get any events. These things may be somewhat rare but do happen (e.g. browser safe search uses a bloom filter for URL lookups, which has had false positives in the past due to this).

That's with fp=0.01 (as per your patch). Tweaking this value I get fp=0.001 50042, fp=0.0001 50008, fp=0.00001 50k.

Tweaking this value to 0.00001 makes appends (and by extension, the already slow delete) about twice as slow, to 12ms, and it will use 146K of memory.

Deleting 100 files would mean spending 1.2 seconds just on creating new bloom filters. Deleting 1,000 files would be 12 seconds. Yikes!

Unless I spectacularly missed something, the conclusion seems obvious given the above: bloom filters are absolutely not suitable for the general case, and they are probably not suitable for your use case either. It will regress performance significantly in the hopes that it may perhaps save some memory and offer no advantages at beyond that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests