tywkeene/go-fsevents

Missing events. Systemic problem?

markschmid opened this issue · 3 comments

Here's something that confuses me:
When a new directory in a watched directory is created, we add a descriptor to watch that new directory. So far so good, but what happens if files are added to that new directory faster than / before the new descriptor (which makes sure to watch the directory for events) has been correctly installed? Do we miss those files?

Not sure what to make of it but we are actually indeed missing some events from time to time, especially if multiple directories are created at the same time (a few ms apart) and files are rapidly added to those directories.

Apologies for the super late response, holidays and starting a new job have held me back from getting to side projects. I was thinking about this today and I'm planning on exploring some benchmarking this weekend to see if I can track down the behavior you've described.

If you want you can also explore a bit and send a pull request.

Thanks

I was just reading through the code to find a solution to a completely different problem when I noticed a bug in the code that would result in missing events. Came to the issue tracker to see if any such issue was reported and stumbled across this.

Bottom line the problem is in how ReadSingleEvent() works. When ReadSingleEvent() is called, it creates a new buffer, and reads from the file descriptor into this buffer. After handling a single event, the function returns and the buffer is discarded. However when you read from the file descriptor, there are multiple pending events, and your buffer is big enough to hold them, the buffer will contain multiple events. Thus this buffer needs to be preserved and reused between calls to ReadSingleEvent().

Thankfully the fix should be really easy. There is never any risk of the buffer containing a partial event as the inotify system will only provide complete events. So the buffer just needs to be put in the Watcher struct, and any call to ReadSingleEvent() should use the buffer if it's not empty.

Edit / addendum:
I would also strongly recommend this buffer be sized much larger than the minimum to hold a single event (which is what it currently is). For watchers with a high throughput of events, the frequent reads can have a performance impact.

@markschmid @phemmer There was a recent PR #28 that may have (should) solved this problem. Letting you know so you might be able to verify.