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

refactor(core): Implement our own polling file watcher #7674

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

timoffex
Copy link
Contributor

@timoffex timoffex commented May 20, 2024

Description

Replaces the radovskyb/watcher by our own implementation with important advantages.

In the following snippet, onChange1 is guaranteed to be invoked and onChange2 is guaranteed to not be invoked:

watcher.Watch(path, onChange1)
modifyFile(path) // assume this changes mtime or file size
watcher.Watch(path, onChange2)

Another difference is that Watch and WatchTree work even if the path does not point to an existing file or directory.

The parallelism is also different: instead of a single goroutine looping over all watched files, there is one goroutine per watched path. This is primarily for simplicity, but it also prevents a watcher for a large file tree from slowing down other watchers. Though there is only one goroutine per path, that goroutine may own multiple callbacks which minimizes the number of Stat() calls per path.

Created using spr 1.3.5
Created using spr 1.3.5
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 90.56604% with 15 lines in your changes missing coverage. Please review.

Project coverage is 73.01%. Comparing base (6bc85c2) to head (635cad9).
Report is 44 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7674      +/-   ##
==========================================
+ Coverage   72.81%   73.01%   +0.19%     
==========================================
  Files         491      502      +11     
  Lines       55565    54240    -1325     
==========================================
- Hits        40462    39604     -858     
+ Misses      14681    14226     -455     
+ Partials      422      410      -12     
Flag Coverage Δ
func 41.36% <ø> (+0.04%) ⬆️
system 63.53% <70.19%> (+0.03%) ⬆️
unit 52.08% <89.30%> (+1.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
core/internal/waiting/waiting.go 81.39% <100.00%> (+9.30%) ⬆️
core/internal/waitingtest/fakestopwatch.go 79.41% <100.00%> (ø)
core/internal/watcher/filemodtoken.go 100.00% <100.00%> (ø)
core/internal/watcher/watcher.go 100.00% <ø> (ø)
core/pkg/server/tb.go 22.41% <0.00%> (+2.69%) ⬆️
core/internal/watcher/watcherimpl.go 95.83% <95.83%> (ø)
core/internal/watchertest/watchertest.go 50.00% <33.33%> (ø)
core/internal/watcher/treewatcher.go 91.48% <91.48%> (ø)

... and 145 files with indirect coverage changes

Created using spr 1.3.5
Created using spr 1.3.5
Created using spr 1.3.5
Created using spr 1.3.5
@timoffex timoffex marked this pull request as ready for review May 20, 2024 20:44
@timoffex timoffex requested a review from a team as a code owner May 20, 2024 20:44
Created using spr 1.3.5
Created using spr 1.3.5
Created using spr 1.3.5
@timoffex timoffex marked this pull request as draft May 28, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant