inotify: add recursive watcher #472
Merged
+227
−59
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 27, 2022
arp242
added a commit
that referenced
this pull request
Jul 30, 2022
This rewrites quite a lot of tests to be much more easily readable. While working on #472 I wanted to check "how do renames behave now?", and found this quite hard as most test cases were >90% "plumbing", and seeing "what file operations does this do?" and "what events do we get?" was not very easy. So refactor the lot, based on some work I did in #472: - Add a bunch of "shell-like" helper functions so you're not forever typing error checks, filepath.Join(), and eventSeparator(). Just touch(t, tmp, "file") will create a file in tmp. - Add eventCollector type which will collect all events in in a slice, replacing the previous "counter". This also ensures that the Watcher is closed within a second (this removes a lot of duplicate code). This is also much more precise than merely counting events; before random events could get emitted but if you weren't counting those then you'd never know. Downside is that some tests are a bit more flaky now as some behaviours are not always consistent in various edge cases; these are pre-existing bugs. - Add Events (plural) type (only for tests), and helper function to create this from a string like: REMOVE /link CREATE /link WRITE /link Which makes seeing which events are received, diffing them, etc. much easier. - Add Parallel() for most tests; reduces runtime on my system from ~12 seconds to ~6 seconds. All in all it reduces the integrations_test.go from 1279 lines to 405 lines and it's quite easy to see which events are expected for which operations, which should make things a lot easier going forward.
Merged
horahoradev
reviewed
Jul 31, 2022
horahoradev
reviewed
Jul 31, 2022
arp242
added a commit
that referenced
this pull request
Jul 31, 2022
Rewrite tests This rewrites quite a lot of tests to be much more easily readable. While working on #472 I wanted to check "how do renames behave now?", and found this quite hard as most test cases were >90% "plumbing", and seeing "what file operations does this do?" and "what events do we get?" was not very easy. So refactor the lot, based on some work I did in #472: - Add a bunch of "shell-like" helper functions so you're not forever typing error checks, filepath.Join(), and eventSeparator(). Just touch(t, tmp, "file") will create a file in tmp. - Add eventCollector type which will collect all events in in a slice, replacing the previous "counter". This also ensures that the Watcher is closed within a second (this removes a lot of duplicate code). This is also much more precise than merely counting events; before random events could get emitted but if you weren't counting those then you'd never know. Downside is that some tests are a bit more flaky now as some behaviours are not always consistent in various edge cases; these are pre-existing bugs. - Add Events (plural) type (only for tests), and helper function to create this from a string like: REMOVE /link CREATE /link WRITE /link Which makes seeing which events are received, diffing them, etc. much easier. - Add Parallel() for most tests; reduces runtime on my system from ~12 seconds to ~6 seconds. All in all it reduces the integrations_test.go from 1279 lines to 405 lines and it's quite easy to see which events are expected for which operations, which should make things a lot easier going forward.
arp242
force-pushed
the
inotify-recurse
branch
3 times, most recently
from
a21593f
to
1a30f16
Compare
arp242
added a commit
that referenced
this pull request
Dec 20, 2022
Create a new watcher type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Dec 20, 2022
Create a new watcher type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Dec 20, 2022
Create a new watcher type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
This was referenced Dec 20, 2022
Draft
arp242
added a commit
that referenced
this pull request
Dec 21, 2022
Create a new watcher type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Dec 21, 2022
Create a new watches type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Dec 22, 2022
Create a new watches type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Jan 12, 2023
Create a new watches type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
added a commit
that referenced
this pull request
Jan 14, 2023
Create a new watches type to keep track of the watches instead of keeping two maps on the Watcher and accessing these directly. This makes the bookkeeping a bit easier to follow, and we no longer need to worry about locking map access as the watcher type takes care of that now. Came up in #472 where I want to keep track if a path was added recursively, and this makes that a bit easier. Also seems a bit faster: BenchmarkWatch-2 903709 7122 ns/op 194 B/op 3 allocs/op BenchmarkWatch-2 923980 6322 ns/op 196 B/op 3 allocs/op Although that benchmark is very simply and only tests one code path; just want to make sure it's not a horrible regression.
arp242
force-pushed
the
inotify-recurse
branch
4 times, most recently
from
d3727c3
to
f4c00e5
Compare
This adds a recursive watcher for inotify.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a recursive watcher for inotify.
Updates: #18