Skip to content

Conversation

andreynering
Copy link
Member

@andreynering andreynering commented Feb 8, 2025

We're migrating from watcher to fsnotify, which is more performant and will avoid issues with high CPU usage.

This will close the following issues:

TODO:

  • fsnotify recommends us to watch directory instead of individual files.
    • Add a function to internal/fingerprint to return only the directories and watch for those.
    • When an event comes, we'll need to double check if the file is included in sources.
  • Mark the --interval flag and interval: setting as deprecated. We should probably emit a warning if set, as from now on these will be ignored.
    • I actually reconsidered this and I'm reusing the interval setting as the time we wait for suplicated events. Changed default from 5 seconds to 100ms.
  • Dedup duplicated events

Possible post-launch improvements:

@andreynering andreynering added the area: watcher Changes related to the Taskfile watcher. label Feb 8, 2025
@andreynering andreynering self-assigned this Feb 8, 2025
@andreynering andreynering changed the title feat(watcher): migrate to fsnotify feat(watch): migrate to fsnotify Feb 8, 2025
@andreynering andreynering force-pushed the fsnotify branch 4 times, most recently from 07524dd to 09f9211 Compare February 9, 2025 23:54
@andreynering andreynering marked this pull request as ready for review February 18, 2025 01:24
Copy link
Member

Choose a reason for hiding this comment

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

I'm having issues where if a directory is empty when I start the watcher and then create files, nothing will be detected. If I create any file before starting the watcher, then other new files are detected.

I'm also not able to get deletes to work (on Linux). When I remove a file, I just get something like this:

task: received watch event: WRITE         ".../testdata/watcher_interval/src/foo.txt"
task: skipped for file not in sources: src/foo.txt

But it does not start the task.

Copy link

Choose a reason for hiding this comment

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

I'm having issues where if a directory is empty when I start the watcher and then create files, nothing will be detected. If I create any file before starting the watcher, then other new files are detected.

I've observed the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still going to work on this issue another day.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pd93 @radcool I just pushed fixes for both of the issues you reported.

@andreynering andreynering force-pushed the fsnotify branch 2 times, most recently from 96a6691 to f76d72f Compare February 23, 2025 18:54
@andreynering andreynering force-pushed the fsnotify branch 2 times, most recently from 614c656 to 13168c3 Compare March 9, 2025 01:17
@andreynering andreynering requested review from pd93 and radcool March 9, 2025 01:19
Copy link
Member

@pd93 pd93 left a comment

Choose a reason for hiding this comment

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

This is looking good now! (At least on Linux). I can't see any unexpected behaviour and the previously mentioned issues seem to be fixed. Performance also seems much better than before.

I can't speak to the remaining points around ulimits on MacOS (I don't own any Apple hardware), but it seems like regardless of that issue, this is a big step forwards and those things could be addressed separately if required. I think for most users, the default ulimit will be enough and CGO seems like a line we can't cross in Task anyway.

@andreynering
Copy link
Member Author

I'm merging this as this is an significant improvement on it already. We can still do further improvements with time. 🙂

@andreynering andreynering merged commit 0d5f2b5 into main Mar 22, 2025
14 checks passed
@andreynering andreynering deleted the fsnotify branch March 22, 2025 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: watcher Changes related to the Taskfile watcher.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants