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.

Sign up for GitHub

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

Simplify file watch #44

Closed
wants to merge 4 commits into from
Closed

Simplify file watch #44

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63d6383
file: simplify the watch management logic
ffrank May 18, 2016
7068a92
file: simplify watch handling, on each remove, search the deepest exi…
ffrank May 18, 2016
cf3ce2e
file: just reset the watch on every event
ffrank May 19, 2016
3eb4a2f
file: slightly simplify watch error handling
ffrank May 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 64 additions & 81 deletions file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"encoding/gob"
"io"
"log"
"math"
"os"
"path"
"strings"
Expand Down Expand Up @@ -114,121 +113,105 @@ func (obj *FileRes) Watch(processChan chan Event) {
//var recursive bool = false
//var isdir = (obj.GetPath()[len(obj.GetPath())-1:] == "/") // dirs have trailing slashes
//log.Printf("IsDirectory: %v", isdir)
var safename = path.Clean(obj.GetPath()) // no trailing slash
var cleanObjPath = path.Clean(obj.GetPath()) // no trailing slash

watcher, err := fsnotify.NewWatcher()
if err != nil {
log.Fatal(err)
}
defer watcher.Close()

patharray := PathSplit(safename) // tokenize the path
var index = len(patharray) // starting index
var current string // current "watcher" location
var deltaDepth int // depth delta between watcher and event
var send = false // send event?
patharray := PathSplit(cleanObjPath)[1:] // tokenize the path
var objDepth = len(patharray) // total elements in the path, will not change
var watchDepth = len(patharray) // elements in the path to the current watch
var eventDepth int // elements in the path to what triggered the most recent event
var send = false // send event?
var exit = false
var dirty = false
var watchPath string
var watching = false

for {
current = strings.Join(patharray[0:index], "/")
if current == "" { // the empty string top is the root dir ("/")
current = "/"
}
watchPath = "/" + strings.Join(patharray[0:watchDepth], "/")

if DEBUG {
log.Printf("File[%v]: Watching: %v", obj.GetName(), current) // attempting to watch...
log.Printf("File[%v]: Watching: %v", obj.GetName(), watchPath) // attempting to watch...
}
// initialize in the loop so that we can reset on rm-ed handles
err = watcher.Add(current)
if err != nil {
if DEBUG {
log.Printf("File[%v]: watcher.Add(%v): Error: %v", obj.GetName(), current, err)
}
if err == syscall.ENOENT {
index-- // usually not found, move up one dir
} else if err == syscall.ENOSPC {
// XXX: occasionally: no space left on device,
// XXX: probably due to lack of inotify watches
log.Printf("%v[%v]: Out of inotify watches!", obj.Kind(), obj.GetName())
log.Fatal(err)
} else {
log.Printf("Unknown file[%v] error:", obj.Name)

if !watching {
err = watcher.Add(watchPath)
if err != nil {
if DEBUG {
log.Printf("File[%v]: watcher.Add(%v): Error: %v", obj.GetName(), watchPath, err)
}
if err == syscall.ENOENT {
watchDepth-- // usually not found, move up one dir
if watchDepth < 0 {
log.Fatal("somehow trying to watch file above the fs root")
}
continue
}
if err == syscall.ENOSPC {
// XXX: occasionally: no space left on device,
// XXX: probably due to lack of inotify watches
log.Printf("%v[%v]: Out of inotify watches!", obj.Kind(), obj.GetName())
} else {
log.Printf("Unknown file[%v] error:", obj.Name)
}
log.Fatal(err)
}
index = int(math.Max(1, float64(index)))
continue
watching = true
}

obj.SetState(resStateWatching) // reset
select {
case event := <-watcher.Events:
if DEBUG {
log.Printf("File[%v]: Watch(%v), Event(%v): %v", obj.GetName(), current, event.Name, event.Op)
log.Printf("File[%v]: Watch(%v), Event(%v): %v", obj.GetName(), watchPath, event.Name, event.Op)
}
cuuid.SetConverged(false) // XXX: technically i can detect if the event is erroneous or not first
// the deeper you go, the bigger the deltaDepth is...
// this is the difference between what we're watching,
// and the event... doesn't mean we can't watch deeper
if current == event.Name {
deltaDepth = 0 // i was watching what i was looking for

} else if HasPathPrefix(event.Name, current) {
deltaDepth = len(PathSplit(current)) - len(PathSplit(event.Name)) // -1 or less

} else if HasPathPrefix(current, event.Name) {
deltaDepth = len(PathSplit(event.Name)) - len(PathSplit(current)) // +1 or more

} else {
if !HasPathPrefix(event.Name, cleanObjPath) && !HasPathPrefix(cleanObjPath, event.Name) {
// TODO different watchers get each others events!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this one. The behavior you describe in that bug report might be exhibited my mgmt because of the whole watching logic. Here's what may have happened, I figure:

  1. You run mgmt with the file1.yaml example, but no /tmp/mgmt. /tmp is watched by all four file resources.
  2. You create /tmp/mgmt. All four resources race to start watching it and run a CheckApply.
  3. At the time f3 is created, the f2 resource may still be watching /tmp/mgmt, so the f3 creation event ends up in its notify queue.
  4. After the f2 resource has consumed the /tmp/mgmt event and descended in to watch /tmp/mgmt/f2, it takes the f3 event out of its queue.
  5. Boom! :)

Something along those lines. It seems more plausible than a library or kernel bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen this as well? Thoughts?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the correct behaviour of the library is to only give you events you've asked for, but in the scenario that it gives us events we don't want (as seen here) we just ignore them.

// https://github.com/go-fsnotify/fsnotify/issues/95
// this happened with two values such as:
// event.Name: /tmp/mgmt/f3 and current: /tmp/mgmt/f2
// event.Name: /tmp/mgmt/f3 and watchPath: /tmp/mgmt/f2
if DEBUG {
log.Printf("File[%v]: ignoring event, it's not related", obj.GetName)
}
continue
}
//log.Printf("The delta depth is: %v", deltaDepth)

// if we have what we wanted, awesome, send an event...
if event.Name == safename {
//log.Println("Event!")
// FIXME: should all these below cases trigger?
if DEBUG {
log.Printf("File[%v]: event depth %v, watch depth %v", obj.GetName(), eventDepth, watchDepth)
}

cuuid.SetConverged(false)
eventDepth = len(PathSplit(event.Name)[1:])
// reset the watch
watcher.Remove(watchPath)
watchDepth = objDepth
watching = false

if eventDepth == objDepth {
// this event was triggered by the managed file: send an event
send = true
dirty = true
}

// file removed, move the watch upwards
if deltaDepth >= 0 && (event.Op&fsnotify.Remove == fsnotify.Remove) {
//log.Println("Removal!")
watcher.Remove(current)
index--
}

// we must be a parent watcher, so descend in
if deltaDepth < 0 {
watcher.Remove(current)
index++
}

// if safename starts with event.Name, we're above, and no event should be sent
} else if HasPathPrefix(safename, event.Name) {
//log.Println("Above!")
// were we watching an ancestor?
if eventDepth < objDepth {

if deltaDepth >= 0 && (event.Op&fsnotify.Remove == fsnotify.Remove) {
log.Println("Removal!")
watcher.Remove(current)
index--
}

if deltaDepth < 0 {
log.Println("Parent!")
if PathPrefixDelta(safename, event.Name) == 1 { // we're the parent dir
send = true
dirty = true
}
watcher.Remove(current)
index++
if event.Op&fsnotify.Remove == 0 && objDepth-eventDepth == 1 {
// event from the parent of the managed file
// that is not its removal
log.Printf("File[%v]: Parent event!", obj.GetName())
send = true
dirty = true
}
}

// if event.Name startswith safename, send event, we're already deeper
} else if HasPathPrefix(event.Name, safename) {
// event from within a managed directory tree?
if eventDepth > objDepth {
//log.Println("Event2!")
send = true
dirty = true
Expand Down