*: reflow comments to 80 characters
This reformats the entire Metropolis codebase to have comments no longer
than 80 characters, implementing CR/66.
This has been done half manually, as we don't have a good integration
between commentwrap/Bazel, but that can be implemented if we decide to
go for this tool/limit.
Change-Id: If1fff0b093ef806f5dc00551c11506e8290379d0
diff --git a/metropolis/pkg/supervisor/supervisor_processor.go b/metropolis/pkg/supervisor/supervisor_processor.go
index 965a667..5fa759e 100644
--- a/metropolis/pkg/supervisor/supervisor_processor.go
+++ b/metropolis/pkg/supervisor/supervisor_processor.go
@@ -24,11 +24,13 @@
"time"
)
-// The processor maintains runnable goroutines - ie., when requested will start one, and then once it exists it will
-// record the result and act accordingly. It is also responsible for detecting and acting upon supervision subtrees that
-// need to be restarted after death (via a 'GC' process)
+// The processor maintains runnable goroutines - ie., when requested will start
+// one, and then once it exists it will record the result and act accordingly.
+// It is also responsible for detecting and acting upon supervision subtrees
+// that need to be restarted after death (via a 'GC' process)
-// processorRequest is a request for the processor. Only one of the fields can be set.
+// processorRequest is a request for the processor. Only one of the fields can
+// be set.
type processorRequest struct {
schedule *processorRequestSchedule
died *processorRequestDied
@@ -40,7 +42,8 @@
dn string
}
-// processorRequestDied is a signal from a runnable goroutine that the runnable has died.
+// processorRequestDied is a signal from a runnable goroutine that the runnable
+// has died.
type processorRequestDied struct {
dn string
err error
@@ -57,8 +60,10 @@
// Waiters waiting for the GC to be settled.
var waiters []chan struct{}
- // The GC will run every millisecond if needed. Any time the processor requests a change in the supervision tree
- // (ie a death or a new runnable) it will mark the state as dirty and run the GC on the next millisecond cycle.
+ // The GC will run every millisecond if needed. Any time the processor
+ // requests a change in the supervision tree (ie a death or a new runnable)
+ // it will mark the state as dirty and run the GC on the next millisecond
+ // cycle.
gc := time.NewTicker(1 * time.Millisecond)
defer gc.Stop()
clean := true
@@ -85,7 +90,8 @@
clean = true
cleanCycles += 1
- // This threshold is somewhat arbitrary. It's a balance between test speed and test reliability.
+ // This threshold is somewhat arbitrary. It's a balance between
+ // test speed and test reliability.
if cleanCycles > 50 {
for _, w := range waiters {
close(w)
@@ -109,8 +115,9 @@
}
}
-// processKill cancels all nodes in the supervision tree. This is only called right before exiting the processor, so
-// they do not get automatically restarted.
+// processKill cancels all nodes in the supervision tree. This is only called
+// right before exiting the processor, so they do not get automatically
+// restarted.
func (s *supervisor) processKill() {
s.mu.Lock()
defer s.mu.Unlock()
@@ -138,7 +145,8 @@
}
}
-// processSchedule starts a node's runnable in a goroutine and records its output once it's done.
+// processSchedule starts a node's runnable in a goroutine and records its
+// output once it's done.
func (s *supervisor) processSchedule(r *processorRequestSchedule) {
s.mu.Lock()
defer s.mu.Unlock()
@@ -169,8 +177,9 @@
}()
}
-// processDied records the result from a runnable goroutine, and updates its node state accordingly. If the result
-// is a death and not an expected exit, related nodes (ie. children and group siblings) are canceled accordingly.
+// processDied records the result from a runnable goroutine, and updates its
+// node state accordingly. If the result is a death and not an expected exit,
+// related nodes (ie. children and group siblings) are canceled accordingly.
func (s *supervisor) processDied(r *processorRequestDied) {
s.mu.Lock()
defer s.mu.Unlock()
@@ -195,14 +204,16 @@
break
}
- // Simple case: the context was canceled and the returned error is the context error.
+ // Simple case: the context was canceled and the returned error is the
+ // context error.
if err := ctx.Err(); err != nil && perr == err {
// Mark the node as canceled successfully.
n.state = nodeStateCanceled
return
}
- // Otherwise, the Runnable should not have died or quit. Handle accordingly.
+ // Otherwise, the Runnable should not have died or quit. Handle
+ // accordingly.
err := r.err
// A lack of returned error is also an error.
if err == nil {
@@ -225,27 +236,33 @@
continue
}
sibling := n.parent.children[name]
- // TODO(q3k): does this need to run in a goroutine, ie. can a context cancel block?
+ // TODO(q3k): does this need to run in a goroutine, ie. can a
+ // context cancel block?
sibling.ctxC()
}
}
}
-// processGC runs the GC process. It's not really Garbage Collection, as in, it doesn't remove unnecessary tree nodes -
-// but it does find nodes that need to be restarted, find the subset that can and then schedules them for running.
-// As such, it's less of a Garbage Collector and more of a Necromancer. However, GC is a friendlier name.
+// processGC runs the GC process. It's not really Garbage Collection, as in, it
+// doesn't remove unnecessary tree nodes - but it does find nodes that need to
+// be restarted, find the subset that can and then schedules them for running.
+// As such, it's less of a Garbage Collector and more of a Necromancer.
+// However, GC is a friendlier name.
func (s *supervisor) processGC() {
s.mu.Lock()
defer s.mu.Unlock()
- // The 'GC' serves is the main business logic of the supervision tree. It traverses a locked tree and tries to
- // find subtrees that must be restarted (because of a DEAD/CANCELED runnable). It then finds which of these
- // subtrees that should be restarted can be restarted, ie. which ones are fully recursively DEAD/CANCELED. It
- // also finds the smallest set of largest subtrees that can be restarted, ie. if there's multiple DEAD runnables
- // that can be restarted at once, it will do so.
+ // The 'GC' serves is the main business logic of the supervision tree. It
+ // traverses a locked tree and tries to find subtrees that must be
+ // restarted (because of a DEAD/CANCELED runnable). It then finds which of
+ // these subtrees that should be restarted can be restarted, ie. which ones
+ // are fully recursively DEAD/CANCELED. It also finds the smallest set of
+ // largest subtrees that can be restarted, ie. if there's multiple DEAD
+ // runnables that can be restarted at once, it will do so.
// Phase one: Find all leaves.
- // This is a simple DFS that finds all the leaves of the tree, ie all nodes that do not have children nodes.
+ // This is a simple DFS that finds all the leaves of the tree, ie all nodes
+ // that do not have children nodes.
leaves := make(map[string]bool)
queue := []*node{s.root}
for {
@@ -264,14 +281,17 @@
}
}
- // Phase two: traverse tree from node to root and make note of all subtrees that can be restarted.
- // A subtree is restartable/ready iff every node in that subtree is either CANCELED, DEAD or DONE.
- // Such a 'ready' subtree can be restarted by the supervisor if needed.
+ // Phase two: traverse tree from node to root and make note of all subtrees
+ // that can be restarted.
+ // A subtree is restartable/ready iff every node in that subtree is either
+ // CANCELED, DEAD or DONE. Such a 'ready' subtree can be restarted by the
+ // supervisor if needed.
// DNs that we already visited.
visited := make(map[string]bool)
// DNs whose subtrees are ready to be restarted.
- // These are all subtrees recursively - ie., root.a.a and root.a will both be marked here.
+ // These are all subtrees recursively - ie., root.a.a and root.a will both
+ // be marked here.
ready := make(map[string]bool)
// We build a queue of nodes to visit, starting from the leaves.
@@ -299,17 +319,20 @@
}
}
- // If no decision about children is available, it means we ended up in this subtree through some shorter path
- // of a shorter/lower-order leaf. There is a path to a leaf that's longer than the one that caused this node
- // to be enqueued. Easy solution: just push back the current element and retry later.
+ // If no decision about children is available, it means we ended up in
+ // this subtree through some shorter path of a shorter/lower-order
+ // leaf. There is a path to a leaf that's longer than the one that
+ // caused this node to be enqueued. Easy solution: just push back the
+ // current element and retry later.
if !allVisited {
// Push back to queue and wait for a decision later.
queue = append(queue, cur)
continue
}
- // All children have been visited and we have an idea about whether they're ready/restartable. All of the node's
- // children must be restartable in order for this node to be restartable.
+ // All children have been visited and we have an idea about whether
+ // they're ready/restartable. All of the node's children must be
+ // restartable in order for this node to be restartable.
childrenReady := true
for _, c := range cur.children {
if !ready[c.dn()] {
@@ -318,7 +341,8 @@
}
}
- // In addition to children, the node itself must be restartable (ie. DONE, DEAD or CANCELED).
+ // In addition to children, the node itself must be restartable (ie.
+ // DONE, DEAD or CANCELED).
curReady := false
switch cur.state {
case nodeStateDone:
@@ -329,7 +353,8 @@
curReady = true
}
- // Note down that we have an opinion on this node, and note that opinion down.
+ // Note down that we have an opinion on this node, and note that
+ // opinion down.
visited[curDn] = true
ready[curDn] = childrenReady && curReady
@@ -339,15 +364,17 @@
}
}
- // Phase 3: traverse tree from root to find largest subtrees that need to be restarted and are ready to be
- // restarted.
+ // Phase 3: traverse tree from root to find largest subtrees that need to
+ // be restarted and are ready to be restarted.
// All DNs that need to be restarted by the GC process.
want := make(map[string]bool)
- // All DNs that need to be restarted and can be restarted by the GC process - a subset of 'want' DNs.
+ // All DNs that need to be restarted and can be restarted by the GC process
+ // - a subset of 'want' DNs.
can := make(map[string]bool)
- // The set difference between 'want' and 'can' are all nodes that should be restarted but can't yet (ie. because
- // a child is still in the process of being canceled).
+ // The set difference between 'want' and 'can' are all nodes that should be
+ // restarted but can't yet (ie. because a child is still in the process of
+ // being canceled).
// DFS from root.
queue = []*node{s.root}
@@ -366,14 +393,16 @@
// If it should be restarted and is ready to be restarted...
if want[cur.dn()] && ready[cur.dn()] {
- // And its parent context is valid (ie hasn't been canceled), mark it as restartable.
+ // And its parent context is valid (ie hasn't been canceled), mark
+ // it as restartable.
if cur.parent == nil || cur.parent.ctx.Err() == nil {
can[cur.dn()] = true
continue
}
}
- // Otherwise, traverse further down the tree to see if something else needs to be done.
+ // Otherwise, traverse further down the tree to see if something else
+ // needs to be done.
for _, c := range cur.children {
queue = append(queue, c)
}
@@ -383,13 +412,15 @@
for dn, _ := range can {
n := s.nodeByDN(dn)
- // Only back off when the node unexpectedly died - not when it got canceled.
+ // Only back off when the node unexpectedly died - not when it got
+ // canceled.
bo := time.Duration(0)
if n.state == nodeStateDead {
bo = n.bo.NextBackOff()
}
- // Prepare node for rescheduling - remove its children, reset its state to new.
+ // Prepare node for rescheduling - remove its children, reset its state
+ // to new.
n.reset()
s.ilogger.Infof("rescheduling supervised node %s with backoff %s", dn, bo.String())