Issue #186: runtime error: integer divide by zero

When fabio is presented with more than 200 targets for a single
route then a rounding error had the effect that no target would
get traffic which subsequently lead to a division by zero error
in a code path which did not expect an empty list of targets.

This change modifies the distribution algorithm as follows:

* The case where all targets receive an equal amount of traffic
has now a fast path which just returns the list of targets.

* For the other case a larger ring of 10.000 slots is always
used to achieve a certain amount of accuracy. In addition, all
targets that have a non-zero weight will use at least one slot
to work around rounding issues and the fact that some servers
would not receive traffic although they are alive and healthy.

This should guarantee an unlimited number of instances for a
single route.

route/picker.go

@@ -45,4 +45,9 @@ func rrPicker(r *Route) *Target {
 // requests are usually handled within several ms we should have enough
 // variation. Within 1 ms we have 1000 µs to distribute among a smaller
 // set of entities (<< 100)
-var randIntn = func(n int) int { return int(time.Now().UnixNano()/int64(time.Microsecond)) % n }
+var randIntn = func(n int) int {
+	if n == 0 {
+		return 0
+	}
+	return int(time.Now().UnixNano()/int64(time.Microsecond)) % n
+}

route/route.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 	"net/url"
+	"reflect"
 	"sort"
 	"strings"
 
@@ -43,10 +44,18 @@ func newRoute(host, path string) *Route {
 }
 
 func (r *Route) addTarget(service string, targetURL *url.URL, fixedWeight float64, tags []string) {
+	// fmt.Printf("addTarget(%q, %q, %f, %v)\n", service, targetURL, fixedWeight, tags)
 	if fixedWeight < 0 {
 		fixedWeight = 0
 	}
 
+	// de-dup existing target
+	for _, t := range r.Targets {
+		if t.Service == service && t.URL.String() == targetURL.String() && t.FixedWeight == fixedWeight && reflect.DeepEqual(t.Tags, tags) {
+			return
+		}
+	}
+
 	name, err := metrics.TargetName(service, r.Host, r.Path, targetURL)
 	if err != nil {
 		log.Printf("[ERROR] Invalid metrics name: %s", err)
@@ -130,22 +139,12 @@ func contains(src, dst []string) bool {
 	return true
 }
 
-// targetWeight returns how often target is in wTargets.
-func (r *Route) targetWeight(targetURL string) (n int) {
-	for _, t := range r.wTargets {
-		if t.URL.String() == targetURL {
-			n++
-		}
-	}
-	return n
-}
-
 func (r *Route) TargetConfig(t *Target, addWeight bool) string {
 	s := fmt.Sprintf("route add %s %s %s", t.Service, r.Host+r.Path, t.URL)
 	if addWeight {
-		s += fmt.Sprintf(" weight %2.2f", t.Weight)
+		s += fmt.Sprintf(" weight %2.4f", t.Weight)
 	} else if t.FixedWeight > 0 {
-		s += fmt.Sprintf(" weight %.2f", t.FixedWeight)
+		s += fmt.Sprintf(" weight %.4f", t.FixedWeight)
 	}
 	if len(t.Tags) > 0 {
 		s += fmt.Sprintf(" tags %q", strings.Join(t.Tags, ","))
@@ -166,6 +165,12 @@ func (r *Route) config(addWeight bool) []string {
 	return cfg
 }
 
+// maxSlots defines the maximum number of slots on the ring for
+// weighted round-robin distribution for a single route. Consequently,
+// this then defines the maximum number of separate instances that can
+// serve a single route. maxSlots must be a power of ten.
+const maxSlots = 1e4 // 10000
+
 // weighTargets computes the share of traffic each target receives based
 // on its weight and the weight of the other targets.
 //
@@ -185,6 +190,17 @@ func (r *Route) weighTargets() {
 		}
 	}
 
+	// if there are no targets with fixed weight then each target simply gets
+	// an equal amount of traffic
+	if nFixed == 0 {
+		w := 1.0 / float64(len(r.Targets))
+		for _, t := range r.Targets {
+			t.Weight = w
+		}
+		r.wTargets = r.Targets
+		return
+	}
+
 	// normalize fixed weights up (sumFixed < 1) or down (sumFixed > 1)
 	scale := 1.0
 	if sumFixed > 1 || (nFixed == len(r.Targets) && sumFixed < 1) {
@@ -206,49 +222,68 @@ func (r *Route) weighTargets() {
 		}
 	}
 
-	// Distribute the targets on a ring with N slots. The distance
-	// between two entries for the same target should be N/count slots
-	// apart to achieve even distribution. count is the number of slots the
-	// target should get based on its weight.
-	// To achieve this we first determine count per target and then sort that
-	// from smallest to largest to distribute the targets with lesser weight
-	// more evenly. For that we pick a random starting point on the ring and
-	// move clockwise until we find a free spot. The the next slot is N/count
-	// slots away. If it is occupied we again move clockwise until we find
-	// a free slot.
-
-	// number of slots we want to use and number of slots we will actually use
-	// because of rounding errors
-	gotSlots, wantSlots := 0, 100
-
-	slotCount := make(byN, len(r.Targets))
+	// distribute the targets on a ring suitable for weighted round-robin
+	// distribution
+	//
+	// This is done in two steps:
+	//
+	// Step one determines the necessary ring size to distribute the targets
+	// according to their weight with reasonable accuracy. For example, two
+	// targets with 50% weight fit in a ring of size 2 whereas two targets with
+	// 10% and 90% weight require a ring of size 10.
+	//
+	// To keep it simple we allocate 10000 slots which provides slots to all
+	// targets with at least a weight of 0.01%. In addition, we guarantee that
+	// every target with a weight > 0 gets at least one slot. The case where all
+	// targets get an equal share of traffic is handled earlier so this is for
+	// situations with some fixed weight.
+	//
+	// Step two distributes the targets onto the ring spacing them out evenly
+	// so that iterating over the ring performs the weighted rr distribution.
+	// For example, a 50/50 distribution on a ring of size 10 should be
+	// 0101010101 instead of 0000011111.
+	//
+	// To ensure that targets with smaller weights are properly placed we place them on the ring first
+	// by sorting the targets by slot count.
+	//
+	// TODO(fs): I assume that this is some sort of mathematical problem
+	// (coloring, optimizing, ...) but I don't know which. Happy to make this
+	// more formal, if possible.
+	//
+	slots := make(byN, len(r.Targets))
+	maxSlots, usedSlots := 10000, 0
 	for i, t := range r.Targets {
-		slotCount[i].i = i
-		slotCount[i].n = int(float64(wantSlots)*t.Weight + 0.5)
-		gotSlots += slotCount[i].n
+		n := int(float64(maxSlots) * t.Weight)
+		if n == 0 && t.Weight > 0 {
+			n = 1
+		}
+		slots[i].i = i
+		slots[i].n = n
+		usedSlots += n
 	}
-	sort.Sort(slotCount)
 
-	slots := make([]*Target, gotSlots)
-	for _, c := range slotCount {
-		if c.n <= 0 {
+	// fmt.Println("usedSlots: ", usedSlots)
+	sort.Sort(slots)
+	targets := make([]*Target, usedSlots)
+	for _, s := range slots {
+		if s.n <= 0 {
 			continue
 		}
 
-		next, step := 0, gotSlots/c.n
-		for k := 0; k < c.n; k++ {
+		next, step := 0, usedSlots/s.n
+		for k := 0; k < s.n; k++ {
 			// find the next empty slot
-			for slots[next] != nil {
-				next = (next + 1) % gotSlots
+			for targets[next] != nil {
+				next = (next + 1) % usedSlots
 			}
 
 			// use slot and move to next one
-			slots[next] = r.Targets[c.i]
-			next = (next + step) % gotSlots
+			targets[next] = r.Targets[s.i]
+			next = (next + step) % usedSlots
 		}
 	}
 
-	r.wTargets = slots
+	r.wTargets = targets
 }
 
 type byN []struct{ i, n int }

route/table.go

@@ -119,24 +119,24 @@ func (t Table) AddRoute(service, prefix, target string, weight float64, tags []s
 		return fmt.Errorf("route: invalid target. %s", err)
 	}
 
-	r := newRoute(host, path)
-	r.addTarget(service, targetURL, weight, tags)
-
+	switch {
 	// add new host
-	if t[host] == nil {
+	case t[host] == nil:
+		r := newRoute(host, path)
+		r.addTarget(service, targetURL, weight, tags)
 		t[host] = Routes{r}
-		return nil
-	}
 
 	// add new route to existing host
-	if t[host].find(path) == nil {
+	case t[host].find(path) == nil:
+		r := newRoute(host, path)
+		r.addTarget(service, targetURL, weight, tags)
 		t[host] = append(t[host], r)
 		sort.Sort(t[host])
-		return nil
-	}
 
 	// add new target to existing route
-	t[host].find(path).addTarget(service, targetURL, weight, tags)
+	default:
+		t[host].find(path).addTarget(service, targetURL, weight, tags)
+	}
 
 	return nil
 }