Rework the converged detection and provide a clean interface
The old converged detection was hacked in code, instead of something with a nice interface. This cleans it up, splits it into a separate file, and removes a race condition that happened with the old code. We also take the time to get rid of the ugly Set* methods and replace them all with a single AssociateData method. This might be unnecessary if we can pass in the Converger method at Resource construction. Lastly, and most interesting, we suspend the individual timeout callers when they've already converged, thus reducing unnecessary traffic, and avoiding fast (eg: < 5 second) timers triggering more than once if they stay converged! A quick note on theory for any future readers... What happens if we have --converged-timeout=0 ? Well, for this and any other positive value, it's important to realize that deciding if something is converged is actually a race between if the converged timer will fire and if some random new event will get triggered. This is because there is nothing that can actually predict if or when a new event will happen (eg the user modifying a file). As a result, a race is always inherent, and actually not a negative or "incorrect" algorithm. A future improvement could be to add a global lock to each resource, and to lock all resources when computing if we are converged or not. In practice, this hasn't been necessary. The worst case scenario would be (in theory, because this hasn't been tested) if an event happens *during* the converged calculation, and starts running, the exit command then runs, and the event finishes, but it doesn't get a chance to notify some service to restart. A lock could probably fix this theoretical case.
This commit is contained in:
16
exec.go
16
exec.go
@@ -108,11 +108,12 @@ func (obj *ExecRes) Watch(processChan chan Event) {
|
||||
}
|
||||
obj.SetWatching(true)
|
||||
defer obj.SetWatching(false)
|
||||
cuuid := obj.converger.Register()
|
||||
defer cuuid.Unregister()
|
||||
|
||||
var send = false // send event?
|
||||
var exit = false
|
||||
bufioch, errch := make(chan string), make(chan error)
|
||||
//vertex := obj.GetVertex() // stored with SetVertex
|
||||
|
||||
if obj.WatchCmd != "" {
|
||||
var cmdName string
|
||||
@@ -157,7 +158,7 @@ func (obj *ExecRes) Watch(processChan chan Event) {
|
||||
obj.SetState(resStateWatching) // reset
|
||||
select {
|
||||
case text := <-bufioch:
|
||||
obj.SetConvergedState(resConvergedNil)
|
||||
cuuid.SetConverged(false)
|
||||
// each time we get a line of output, we loop!
|
||||
log.Printf("%v[%v]: Watch output: %s", obj.Kind(), obj.GetName(), text)
|
||||
if text != "" {
|
||||
@@ -165,8 +166,8 @@ func (obj *ExecRes) Watch(processChan chan Event) {
|
||||
}
|
||||
|
||||
case err := <-errch:
|
||||
obj.SetConvergedState(resConvergedNil) // XXX ?
|
||||
if err == nil { // EOF
|
||||
cuuid.SetConverged(false) // XXX ?
|
||||
if err == nil { // EOF
|
||||
// FIXME: add an "if watch command ends/crashes"
|
||||
// restart or generate error option
|
||||
log.Printf("%v[%v]: Reached EOF", obj.Kind(), obj.GetName())
|
||||
@@ -177,14 +178,13 @@ func (obj *ExecRes) Watch(processChan chan Event) {
|
||||
// XXX: how should we handle errors?
|
||||
|
||||
case event := <-obj.events:
|
||||
obj.SetConvergedState(resConvergedNil)
|
||||
cuuid.SetConverged(false)
|
||||
if exit, send = obj.ReadEvent(&event); exit {
|
||||
return // exit
|
||||
}
|
||||
|
||||
case _ = <-TimeAfterOrBlock(obj.ctimeout):
|
||||
obj.SetConvergedState(resConvergedTimeout)
|
||||
obj.converged <- true
|
||||
case _ = <-cuuid.ConvergedTimer():
|
||||
cuuid.SetConverged(true) // converged!
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user