Skip to content

Commit

Permalink
watch: increase the windows watch i/o buffer (#3620)
Browse files Browse the repository at this point in the history
fixes #3556
  • Loading branch information
nicks committed Jul 27, 2020
1 parent 8442eab commit 39544b5
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 8 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ require (
github.com/stretchr/testify v1.6.1
github.com/theupdateframework/notary v0.6.1 // indirect
github.com/tilt-dev/fsevents v0.0.0-20200515134857-2efe37af20de
github.com/tilt-dev/fsnotify v1.4.8-0.20200507235935-249ce517a564
github.com/tilt-dev/fsnotify v1.4.8-0.20200727200623-991e307aab7f
github.com/tilt-dev/localregistry-go v0.0.0-20200615231835-07e386f4ebd7
github.com/tilt-dev/wmclient v0.0.0-20200515134039-dd6b302e2564
github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,8 @@ github.com/tilt-dev/client-go v0.0.0-20200326150806-41017343d309 h1:gqiy2KYVUPRB
github.com/tilt-dev/client-go v0.0.0-20200326150806-41017343d309/go.mod h1:uQSYDYs4WhVZ9i6AIoEZuwUggLVEF64HOD37boKAtF8=
github.com/tilt-dev/fsevents v0.0.0-20200515134857-2efe37af20de h1:tG+nJMUUxV7MJm/MeU1Mw7rvmwN9GWyHMMg+wtf9HS4=
github.com/tilt-dev/fsevents v0.0.0-20200515134857-2efe37af20de/go.mod h1:1jUbPVh7Ani2CSublmvP7+zqTgR06A8Y0MKU9Xr2L5s=
github.com/tilt-dev/fsnotify v1.4.8-0.20200507235935-249ce517a564 h1:X4YiQoUKHSc9sahl33PjXjck7PQdoc7xTzPbFL3eBjI=
github.com/tilt-dev/fsnotify v1.4.8-0.20200507235935-249ce517a564/go.mod h1:c6pAambncyReVORBv+ReH61QHPhZ2ddi0/3QyJO3aF8=
github.com/tilt-dev/fsnotify v1.4.8-0.20200727200623-991e307aab7f h1:A3/0Ild2AvlDohwdHnyHYxcrm4syPBT3olxgQrXLbXw=
github.com/tilt-dev/fsnotify v1.4.8-0.20200727200623-991e307aab7f/go.mod h1:c6pAambncyReVORBv+ReH61QHPhZ2ddi0/3QyJO3aF8=
github.com/tilt-dev/json-patch/v4 v4.8.1 h1:AbrhK3NMDfk/+/oMXz3NcKaCNwHYdhUJMDjBHNOHF5o=
github.com/tilt-dev/json-patch/v4 v4.8.1/go.mod h1:tS8rrXPNPgltwGinFPfBjCwHFLxMa3ozjPngreH2eW0=
github.com/tilt-dev/localregistry-go v0.0.0-20200615231835-07e386f4ebd7 h1:ysHGLJJRVcnG6WoZJt7GGD4hsMSwMxEg7Rxx4fJrSrY=
Expand Down
12 changes: 11 additions & 1 deletion internal/engine/fswatch/watchmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,16 @@ func (w *WatchManager) dispatchFileChangesLoop(
if !ok {
return
}
if err.Error() == fsnotify.ErrEventOverflow.Error() {
if watch.IsWindowsShortReadError(err) {
st.Dispatch(store.NewErrorAction(fmt.Errorf("Windows I/O overflow.\n"+
"You may be able to fix this by setting the env var %s.\n"+
"Current buffer size: %d\n"+
"More details: https://github.com/tilt-dev/tilt/issues/3556\n"+
"Caused by: %v",
watch.WindowsBufferSizeEnvVar,
watch.DesiredWindowsBufferSize(),
err)))
} else if err.Error() == fsnotify.ErrEventOverflow.Error() {
st.Dispatch(store.NewErrorAction(fmt.Errorf("%s\nerror: %v", DetectedOverflowErrMsg, err)))
} else {
st.Dispatch(store.NewErrorAction(err))
Expand All @@ -274,6 +283,7 @@ func (w *WatchManager) dispatchFileChangesLoop(
return

case fsEvents, ok := <-eventsCh:

if !ok {
return
}
Expand Down
20 changes: 20 additions & 0 deletions internal/engine/fswatch/watchmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"path/filepath"
"reflect"
"runtime"
"testing"
"time"

Expand Down Expand Up @@ -149,6 +150,25 @@ func TestWatchManager_PickUpTiltIgnoreChanges(t *testing.T) {
f.AssertActionsContain(actions, filepath.Join("bar", "baz", "foo"))
}

func TestWatchManagerShortRead(t *testing.T) {
f := newWMFixture(t)
defer f.TearDown()

target := model.DockerComposeTarget{Name: "foo"}.
WithBuildPath(".")
f.SetManifestTarget(target)

f.fakeMultiWatcher.Errors <- fmt.Errorf("short read on readEvents()")

action := f.store.WaitForAction(t, reflect.TypeOf(store.ErrorAction{}))
msg := action.(store.ErrorAction).Error.Error()
assert.Contains(t, msg, "short read")
if runtime.GOOS == "windows" {
assert.Contains(t, msg, "https://github.com/tilt-dev/tilt/issues/3556")
}
f.store.ClearActions()
}

type wmFixture struct {
ctx context.Context
cancel func()
Expand Down
23 changes: 23 additions & 0 deletions internal/watch/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ package watch
import (
"expvar"
"fmt"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"

"github.com/tilt-dev/tilt/pkg/logger"
)
Expand Down Expand Up @@ -67,3 +71,22 @@ var _ PathMatcher = EmptyMatcher{}
func NewWatcher(paths []string, ignore PathMatcher, l logger.Logger) (Notify, error) {
return newWatcher(paths, ignore, l)
}

const WindowsBufferSizeEnvVar = "TILT_WATCH_WINDOWS_BUFFER_SIZE"

const defaultBufferSize int = 65536

func DesiredWindowsBufferSize() int {
envVar := os.Getenv(WindowsBufferSizeEnvVar)
if envVar != "" {
size, err := strconv.Atoi(envVar)
if err != nil {
return size
}
}
return defaultBufferSize
}

func IsWindowsShortReadError(err error) bool {
return runtime.GOOS == "windows" && err != nil && strings.Contains(err.Error(), "short read")
}
7 changes: 7 additions & 0 deletions internal/watch/watcher_naive.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ func (d *naiveNotify) Errors() chan error {
func (d *naiveNotify) loop() {
defer close(d.wrappedEvents)
for e := range d.events {
// The Windows fsnotify event stream sometimes gets events with empty names
// that are also sent to the error stream. Hmmmm...
if e.Name == "" {
continue
}

if e.Op&fsnotify.Create != fsnotify.Create {
if d.shouldNotify(e.Name) {
d.wrappedEvents <- FileEvent{e.Name}
Expand Down Expand Up @@ -251,6 +257,7 @@ func newWatcher(paths []string, ignore PathMatcher, l logger.Logger) (*naiveNoti
if err != nil {
return nil, err
}
MaybeIncreaseBufferSize(fsw)

err = fsw.SetRecursive()
isWatcherRecursive := err == nil
Expand Down
9 changes: 9 additions & 0 deletions internal/watch/watcher_nonwin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// +build !windows

package watch

import "github.com/tilt-dev/fsnotify"

func MaybeIncreaseBufferSize(w *fsnotify.Watcher) {
// Not needed on non-windows
}
19 changes: 19 additions & 0 deletions internal/watch/watcher_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// +build windows

package watch

import (
"github.com/tilt-dev/fsnotify"
)

// TODO(nick): I think the ideal API would be to automatically increase the
// size of the buffer when we exceed capacity. But this gets messy,
// because each time we get a short read error, we need to invalidate
// everything we know about the currently changed files. So for now,
// we just provide a way for people to increase the buffer ourselves.
//
// It might also pay to be clever about sizing the buffer
// relative the number of files in the directory we're watching.
func MaybeIncreaseBufferSize(w *fsnotify.Watcher) {
w.SetBufferSize(DesiredWindowsBufferSize())
}
22 changes: 19 additions & 3 deletions vendor/github.com/tilt-dev/fsnotify/windows.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ github.com/theupdateframework/notary/tuf/validation
# github.com/tilt-dev/fsevents v0.0.0-20200515134857-2efe37af20de
## explicit
github.com/tilt-dev/fsevents
# github.com/tilt-dev/fsnotify v1.4.8-0.20200507235935-249ce517a564
# github.com/tilt-dev/fsnotify v1.4.8-0.20200727200623-991e307aab7f
## explicit
github.com/tilt-dev/fsnotify
# github.com/tilt-dev/localregistry-go v0.0.0-20200615231835-07e386f4ebd7
Expand Down

0 comments on commit 39544b5

Please sign in to comment.