From 896e177faa301c9c2441080ba48c9c0efba18e93 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 21 Sep 2018 08:53:38 +0200 Subject: [PATCH 1/2] [watch] Pass the poll interval on the command line Signed-off-by: David Gageot --- cmd/skaffold/app/cmd/cmd.go | 1 + pkg/skaffold/config/options.go | 1 + pkg/skaffold/runner/runner.go | 5 ++--- pkg/skaffold/runner/runner_test.go | 3 +++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/skaffold/app/cmd/cmd.go b/cmd/skaffold/app/cmd/cmd.go index 33eda9664f4..cba2f871f09 100644 --- a/cmd/skaffold/app/cmd/cmd.go +++ b/cmd/skaffold/app/cmd/cmd.go @@ -135,6 +135,7 @@ func setFlagsFromEnvVariables(commands []*cobra.Command) { func AddDevFlags(cmd *cobra.Command) { cmd.Flags().BoolVar(&opts.Cleanup, "cleanup", true, "Delete deployments after dev mode is interrupted") cmd.Flags().StringArrayVarP(&opts.Watch, "watch-image", "w", nil, "Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts.") + cmd.Flags().IntVarP(&opts.WatchPollInterval, "watch-poll-interval", "i", 1000, "Interval (in ms) between two checks for file changes.") } func AddRunDeployFlags(cmd *cobra.Command) { diff --git a/pkg/skaffold/config/options.go b/pkg/skaffold/config/options.go index 4d776c3bf09..6e4ddf7a569 100644 --- a/pkg/skaffold/config/options.go +++ b/pkg/skaffold/config/options.go @@ -31,6 +31,7 @@ type SkaffoldOptions struct { CustomTag string Namespace string Watch []string + WatchPollInterval int } // Labels returns a map of labels to be applied to all deployed diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 34f59afaf87..afbe36d4736 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -44,8 +44,6 @@ import ( "github.com/sirupsen/logrus" ) -const PollInterval = 1000 * time.Millisecond - // ErrorConfigurationChanged is a special error that's returned when the skaffold configuration was changed. var ErrorConfigurationChanged = errors.New("configuration changed") @@ -315,7 +313,8 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1 return nil, errors.Wrap(err, "starting port-forwarder") } - return nil, watcher.Run(ctx, PollInterval, onChange) + pollInterval := time.Duration(r.opts.WatchPollInterval) * time.Millisecond + return nil, watcher.Run(ctx, pollInterval, onChange) } func (r *SkaffoldRunner) shouldWatch(artifact *v1alpha3.Artifact) bool { diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index 96e4f94f797..472546088b1 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -336,6 +336,9 @@ func TestDev(t *testing.T) { Deployer: test.deployer, Tagger: &tag.ChecksumTagger{}, watchFactory: test.watcherFactory, + opts: &config.SkaffoldOptions{ + WatchPollInterval: 100, + }, } _, err := runner.Dev(context.Background(), ioutil.Discard, nil) From 65431994ddb1d0b574ef27b3b513780211d545cd Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 21 Sep 2018 09:29:05 +0200 Subject: [PATCH 2/2] [watch] Debounce rapid file changes Fixes #1004 Signed-off-by: David Gageot --- pkg/skaffold/watch/watch.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/skaffold/watch/watch.go b/pkg/skaffold/watch/watch.go index 7ca5a3c7fbb..56bc9294ca9 100644 --- a/pkg/skaffold/watch/watch.go +++ b/pkg/skaffold/watch/watch.go @@ -65,6 +65,8 @@ func (w *watchList) Run(ctx context.Context, pollInterval time.Duration, onChang ticker := time.NewTicker(pollInterval) defer ticker.Stop() + changedComponents := map[int]bool{} + for { select { case <-ctx.Done(): @@ -72,23 +74,36 @@ func (w *watchList) Run(ctx context.Context, pollInterval time.Duration, onChang case <-ticker.C: changed := 0 - for _, component := range *w { + for i, component := range *w { state, err := stat(component.deps) if err != nil { return errors.Wrap(err, "listing files") } if hasChanged(component.state, state) { - component.onChange() + changedComponents[i] = true component.state = state changed++ } } - if changed > 0 { + // Rapid file changes that are more frequent than the poll interval would trigger + // multiple rebuilds. + // To prevent that, we debounce changes that happen too quickly + // by waiting for a full turn where nothing happens and trigger a rebuild for + // the accumulated changes. + if changed == 0 && len(changedComponents) > 0 { + for i, component := range *w { + if changedComponents[i] { + component.onChange() + } + } + if err := onChange(); err != nil { return errors.Wrap(err, "calling final callback") } + + changedComponents = map[int]bool{} } } }