Skip to content

Commit f01d91e

Browse files
arp242kris-watts-gravwell
arp242
and
kris-watts-gravwell
authored
Add NewBufferedWatcher() (#572)
This PR slightly overhauls the NewWatcher() API so that a list of Options functions can be provided. Currently there is only one configured option but there are a few issues asking for more configuration options so this should allow for an extensible method to add configuration options to NewWatcher. The goals for this PR: 1. Do not change the API in any way that would require any refactor by users of the library. 2. Make all options optional and ensure default behavior is unchanged. 3. Provide the option to configure a userland buffer in the event channel. The motivation for this PR: We make extensive use of the fsnotify library but continually run into situations with older RedHat installations where the kernel fsnotify buffer is not large enough to accommodate high load scenarios like log file rotation and the end user cannot modify the kernel parameters. We have been running a fork that created a userland event channel buffer and found that we are able to deal with very large bursts of fsnotify events without being forced to catch errors and recover. The userland channel buffer will never be as fast as the kernel buffer, and in most cases straight won't be used, but this PR gives users of the library the option to add some padding without changing kernel parameters. --------- Co-authored-by: kris <kris.watts@gravwell.io>
1 parent e545940 commit f01d91e

13 files changed

Lines changed: 360 additions & 165 deletions

‎.circleci/config.yml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ jobs:
2323
command: |
2424
uname -a
2525
go version
26-
go test -parallel 1 -race ./...
26+
FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./...
27+
go test -parallel 1 -race ./...
2728
2829
# iOS
2930
ios:
@@ -48,7 +49,8 @@ jobs:
4849
export PATH=$PATH:/usr/local/Cellar/go/*/bin
4950
uname -a
5051
go version
51-
go test -parallel 1 -race ./...
52+
FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./...
53+
go test -parallel 1 -race ./...
5254
5355
# This is just Linux x86_64; also need to get a Go with GOOS=android, but
5456
# there aren't any pre-built versions of that on the Go site. Idk, disable for
@@ -76,5 +78,6 @@ jobs:
7678
# uname -a
7779
# export PATH=/usr/local/go/bin:$PATH
7880
# go version
79-
# go test -parallel 1 -race ./...
81+
# FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./...
82+
# go test -parallel 1 -race ./...
8083
#

‎.cirrus.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ freebsd_task:
99
# run tests as user "cirrus" instead of root
1010
- pw useradd cirrus -m
1111
- chown -R cirrus:cirrus .
12-
- sudo -u cirrus go test -parallel 1 -race ./...
12+
- FSNOTIFY_BUFFER=4096 sudo --preserve-env=FSNOTIFY_BUFFER -u cirrus go test -parallel 1 -race ./...
13+
- sudo --preserve-env=FSNOTIFY_BUFFER -u cirrus go test -parallel 1 -race ./...

‎.github/workflows/test.yml‎

Lines changed: 70 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -6,71 +6,74 @@ on:
66
branches: ['main', 'aix']
77

88
jobs:
9-
# Test Windows and Linux with the latest Go version and the oldest we support.
10-
test:
9+
linux:
1110
strategy:
1211
fail-fast: false
1312
matrix:
14-
os:
15-
- ubuntu-latest
16-
- windows-latest
17-
go:
18-
- '1.17'
19-
- '1.21'
13+
os: ['ubuntu-latest']
14+
go: ['1.17', '1.21']
2015
runs-on: ${{ matrix.os }}
2116
steps:
22-
- name: checkout
23-
uses: actions/checkout@v3
24-
25-
- name: setup Go
26-
uses: actions/setup-go@v4
17+
- uses: 'actions/checkout@v3'
18+
- uses: 'actions/setup-go@v4'
2719
with:
2820
go-version: ${{ matrix.go }}
21+
- name: test
22+
run: |
23+
FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./...
24+
go test -parallel 1 -race ./...
2925
26+
windows:
27+
strategy:
28+
fail-fast: false
29+
matrix:
30+
os: ['windows-latest']
31+
go: ['1.17', '1.21']
32+
runs-on: ${{ matrix.os }}
33+
steps:
34+
- uses: 'actions/checkout@v3'
35+
- uses: 'actions/setup-go@v4'
36+
with:
37+
go-version: ${{ matrix.go }}
3038
- name: test
3139
run: |
3240
go test -parallel 1 -race ./...
41+
set "FSNOTIFY_BUFFER=4096"
42+
go test -parallel 1 -race ./...
3343
3444
# Test gccgo
35-
testgcc:
36-
runs-on: ubuntu-22.04
37-
name: test (ubuntu-22.04, gccgo 12.1)
45+
gcc:
46+
runs-on: 'ubuntu-22.04'
47+
name: 'test (ubuntu-22.04, gccgo 12.1)'
3848
steps:
39-
- name: checkout
40-
uses: actions/checkout@v3
41-
49+
- uses: 'actions/checkout@v3'
4250
- name: test
4351
run: |
4452
sudo apt-get -y install gccgo-12
45-
go-12 test -parallel 1 ./...
53+
FSNOTIFY_BUFFER=4096 go-12 test -parallel 1 ./...
54+
go-12 test -parallel 1 ./...
4655
4756
# Test only the latest Go version on macOS; we use the macOS builders for BSD
4857
# and illumos, and GitHub doesn't allow many of them to run concurrently. If
4958
# it works on Windows and Linux with Go 1.17, then it probably does on macOS
5059
# too.
51-
testMacOS:
60+
macos:
5261
name: test
5362
strategy:
5463
fail-fast: false
5564
matrix:
56-
os:
57-
- macos-11
58-
- macos-13
59-
go:
60-
- '1.21'
65+
os: ['macos-11', 'macos-13']
66+
go: ['1.21']
6167
runs-on: ${{ matrix.os }}
6268
steps:
63-
- name: checkout
64-
uses: actions/checkout@v3
65-
66-
- name: setup Go
67-
uses: actions/setup-go@v4
69+
- uses: 'actions/checkout@v3'
70+
- uses: 'actions/setup-go@v4'
6871
with:
6972
go-version: ${{ matrix.go }}
70-
7173
- name: test
7274
run: |
73-
go test -parallel 1 -race ./...
75+
FSNOTIFY_BUFFER=4096 go test -parallel 1 -race ./...
76+
go test -parallel 1 -race ./...
7477
7578
# OpenBSD; no -race as the VM doesn't include the comp set.
7679
#
@@ -79,57 +82,64 @@ jobs:
7982
# so should probably look into that first. Go 1.19 is supposed to have a
8083
# much faster race detector, so maybe waiting until we have that is
8184
# enough.
82-
testOpenBSD:
83-
runs-on: macos-12
84-
name: test (openbsd, 1.17)
85+
openbsd:
86+
runs-on: 'macos-12'
87+
timeout-minutes: 30
88+
name: 'test (openbsd, 1.17)'
8589
steps:
86-
- uses: actions/checkout@v3
87-
- name: test (openbsd, 1.17)
88-
id: test
89-
uses: vmactions/openbsd-vm@v0
90+
- uses: 'actions/checkout@v3'
91+
- name: 'test (openbsd, 1.17)'
92+
id: 'openbsd'
93+
uses: 'vmactions/openbsd-vm@v0'
9094
with:
9195
prepare: pkg_add go
9296
run: |
9397
useradd -mG wheel action
94-
su action -c 'go test -parallel 1 ./...'
98+
FSNOTIFY_BUFFER=4096 su action -c 'go test -parallel 1 ./...'
99+
su action -c 'go test -parallel 1 ./...'
95100
96101
# NetBSD
97-
testNetBSD:
102+
netbsd:
98103
runs-on: macos-12
104+
timeout-minutes: 30
99105
name: test (netbsd, 1.20)
100106
steps:
101-
- uses: actions/checkout@v3
102-
- name: test (netbsd, 1.20)
103-
id: test
104-
uses: vmactions/netbsd-vm@v0
107+
- uses: 'actions/checkout@v3'
108+
- name: 'test (netbsd, 1.20)'
109+
id: 'netbsd'
110+
uses: 'vmactions/netbsd-vm@v0'
105111
with:
106112
prepare: pkg_add go
107113
# TODO: no -race for the same reason as OpenBSD (the timing; it does run).
108114
run: |
109115
useradd -mG wheel action
110-
su action -c 'go120 test -parallel 1 ./...'
116+
FSNOTIFY_BUFFER=4096 su action -c 'go120 test -parallel 1 ./...'
117+
su action -c 'go120 test -parallel 1 ./...'
111118
112119
# illumos
113-
testillumos:
120+
illumos:
114121
runs-on: macos-12
122+
timeout-minutes: 30
115123
name: test (illumos, 1.19)
116124
steps:
117-
- uses: actions/checkout@v3
118-
- name: test (illumos, 1.19)
119-
id: test
120-
uses: papertigers/illumos-vm@r38
125+
- uses: 'actions/checkout@v3'
126+
- name: 'test (illumos, 1.19)'
127+
id: 'illumos'
128+
uses: 'papertigers/illumos-vm@r38'
121129
with:
122130
prepare: |
123131
pkg install go-119
124132
run: |
125133
useradd action
126134
export GOCACHE=/tmp/go-cache
127135
export GOPATH=/tmp/go-path
128-
su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...'
136+
FSNOTIFY_BUFFER=4096 su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...'
137+
su action -c '/opt/ooce/go-1.19/bin/go test -parallel 1 ./...'
129138
130139
# Older Debian 6, for old Linux kernels.
131-
testDebian6:
140+
debian6:
132141
runs-on: macos-12
142+
timeout-minutes: 30
133143
name: test (debian6, 1.19)
134144
strategy:
135145
fail-fast: false
@@ -149,16 +159,18 @@ jobs:
149159
with:
150160
go-version: '1.19'
151161

152-
- name: test (debian6, 1.19)
153-
id: test
162+
- name: 'test (debian6, 1.19)'
163+
id: 'debian6'
154164
run: |
155165
cp -f .github/workflows/Vagrantfile.debian6 Vagrantfile
156166
export GOOS=linux
157167
export GOARCH=amd64
158168
for p in $(go list ./...); do
159-
go test -c -o ${p//\//-}.test $p
169+
FSNOTIFY_BUFFER=4096 go test -c -o ${p//\//-}.test $p
170+
go test -c -o ${p//\//-}.test $p
160171
done
161172
vagrant up
162173
for t in *.test; do
163-
vagrant ssh -c "/vagrant/$t -test.parallel 1"
174+
FSNOTIFY_BUFFER=4096 vagrant ssh -c "/vagrant/$t -test.parallel 1"
175+
vagrant ssh -c "/vagrant/$t -test.parallel 1"
164176
done

‎CHANGELOG.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,17 @@ This version of fsnotify needs Go 1.17.
88

99
- illumos: add FEN backend to support illumos and Solaris. ([#371])
1010

11+
- all: add `NewBufferedWatcher()` to use a buffered channel, which can be useful
12+
in cases where you can't control the kernel buffer and receive a large number
13+
of events in bursts. ([#550], [#572])
14+
1115
- all: add `AddWith()`, which is identical to `Add()` but allows passing
1216
options. ([#521])
1317

14-
- windows: allow setting the buffer size with `fsnotify.WithBufferSize()`; the
15-
default of 64K is the highest value that works on all platforms and is enough
16-
for most purposes, but in some cases a highest buffer is needed. ([#521])
18+
- windows: allow setting the ReadDirectoryChangesW() buffer size with
19+
`fsnotify.WithBufferSize()`; the default of 64K is the highest value that
20+
works on all platforms and is enough for most purposes, but in some cases a
21+
highest buffer is needed. ([#521])
1722

1823
### Changes and fixes
1924

@@ -57,7 +62,6 @@ This version of fsnotify needs Go 1.17.
5762
Google AppEngine forbids usage of the unsafe package so the inotify backend
5863
won't compile there.
5964

60-
6165
[#371]: https://github.com/fsnotify/fsnotify/pull/371
6266
[#516]: https://github.com/fsnotify/fsnotify/pull/516
6367
[#518]: https://github.com/fsnotify/fsnotify/pull/518
@@ -67,6 +71,8 @@ This version of fsnotify needs Go 1.17.
6771
[#526]: https://github.com/fsnotify/fsnotify/pull/526
6872
[#528]: https://github.com/fsnotify/fsnotify/pull/528
6973
[#537]: https://github.com/fsnotify/fsnotify/pull/537
74+
[#550]: https://github.com/fsnotify/fsnotify/pull/550
75+
[#572]: https://github.com/fsnotify/fsnotify/pull/572
7076

7177
1.6.0 - 2022-10-13
7278
-------------------

‎backend_fen.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ import (
7777
// Sometimes it will send events for all times, sometimes it will send no
7878
// events, and often only for some files.
7979
//
80-
// The default buffer size is 64K, which is the largest value that is guaranteed
81-
// to work with SMB filesystems. If you have many events in quick succession
82-
// this may not be enough, and you will have to use [WithBufferSize] to increase
83-
// the value.
80+
// The default ReadDirectoryChangesW() buffer size is 64K, which is the largest
81+
// value that is guaranteed to work with SMB filesystems. If you have many
82+
// events in quick succession this may not be enough, and you will have to use
83+
// [WithBufferSize] to increase the value.
8484
type Watcher struct {
8585
// Events sends the filesystem change events.
8686
//
@@ -139,8 +139,19 @@ type Watcher struct {
139139

140140
// NewWatcher creates a new Watcher.
141141
func NewWatcher() (*Watcher, error) {
142+
return NewBufferedWatcher(0)
143+
}
144+
145+
// NewBufferedWatcher creates a new Watcher with a buffered [Events] channel.
146+
//
147+
// The main use-case for this is situations with a very large number of events
148+
// where the kernel buffer size can't be increased (e.g. due to lack of
149+
// permissions). An unbuffered Watcher will perform better for almost all use
150+
// cases, and whenever possible you will be better off increasing the kernel
151+
// buffers instead of adding a large userspace buffer.
152+
func NewBufferedWatcher(sz uint) (*Watcher, error) {
142153
w := &Watcher{
143-
Events: make(chan Event),
154+
Events: make(chan Event, sz),
144155
Errors: make(chan error),
145156
dirs: make(map[string]struct{}),
146157
watches: make(map[string]struct{}),

‎backend_inotify.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ import (
8080
// Sometimes it will send events for all times, sometimes it will send no
8181
// events, and often only for some files.
8282
//
83-
// The default buffer size is 64K, which is the largest value that is guaranteed
84-
// to work with SMB filesystems. If you have many events in quick succession
85-
// this may not be enough, and you will have to use [WithBufferSize] to increase
86-
// the value.
83+
// The default ReadDirectoryChangesW() buffer size is 64K, which is the largest
84+
// value that is guaranteed to work with SMB filesystems. If you have many
85+
// events in quick succession this may not be enough, and you will have to use
86+
// [WithBufferSize] to increase the value.
8787
type Watcher struct {
8888
// Events sends the filesystem change events.
8989
//
@@ -238,6 +238,17 @@ func (w *watches) updatePath(path string, f func(*watch) (*watch, error)) error
238238

239239
// NewWatcher creates a new Watcher.
240240
func NewWatcher() (*Watcher, error) {
241+
return NewBufferedWatcher(0)
242+
}
243+
244+
// NewBufferedWatcher creates a new Watcher with a buffered [Events] channel.
245+
//
246+
// The main use-case for this is situations with a very large number of events
247+
// where the kernel buffer size can't be increased (e.g. due to lack of
248+
// permissions). An unbuffered Watcher will perform better for almost all use
249+
// cases, and whenever possible you will be better off increasing the kernel
250+
// buffers instead of adding a large userspace buffer.
251+
func NewBufferedWatcher(sz uint) (*Watcher, error) {
241252
// Need to set nonblocking mode for SetDeadline to work, otherwise blocking
242253
// I/O operations won't terminate on close.
243254
fd, errno := unix.InotifyInit1(unix.IN_CLOEXEC | unix.IN_NONBLOCK)
@@ -249,7 +260,7 @@ func NewWatcher() (*Watcher, error) {
249260
fd: fd,
250261
inotifyFile: os.NewFile(uintptr(fd), ""),
251262
watches: newWatches(),
252-
Events: make(chan Event),
263+
Events: make(chan Event, sz),
253264
Errors: make(chan error),
254265
done: make(chan struct{}),
255266
doneResp: make(chan struct{}),

0 commit comments

Comments
 (0)