diff --git a/Makefile b/Makefile index cb7656581..85f8b6f1d 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ build: test: $(GO) test -i ./... - $(GO) test -test.timeout 15s `go list ./... | grep -v '/vendor/'` + $(GO) test -test.timeout 30s `go list ./... | grep -v '/vendor/'` gofmt: gofmt -w `find . -type f -name '*.go' | grep -v vendor` diff --git a/route/picker.go b/route/picker.go index fcc58186e..db79703d9 100644 --- a/route/picker.go +++ b/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 +} diff --git a/route/route.go b/route/route.go index 5821f45ea..06662a653 100644 --- a/route/route.go +++ b/route/route.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "net/url" + "reflect" "sort" "strings" @@ -47,6 +48,13 @@ func (r *Route) addTarget(service string, targetURL *url.URL, fixedWeight float6 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 +138,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 +164,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 +189,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 +221,67 @@ 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)) + usedSlots := 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 { + 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 } diff --git a/route/table.go b/route/table.go index 99db657a8..686e48782 100644 --- a/route/table.go +++ b/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 } diff --git a/route/table_weight_test.go b/route/table_weight_test.go index 68a86d9d1..76d4c0cc4 100644 --- a/route/table_weight_test.go +++ b/route/table_weight_test.go @@ -1,193 +1,381 @@ package route import ( + "fmt" + "math" "reflect" + "strconv" "strings" "testing" + "time" ) func TestWeight(t *testing.T) { tests := []struct { - in, out []string - counts []int + desc string + in, out func() []string }{ - { // no fixed weight -> auto distribution - []string{ - `route add svc /foo http://bar:111/`, + { + "dyn weight 1 -> auto distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 1.00`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 1.0000`, + } }, - []int{100}, }, - { // fixed weight 0 -> auto distribution - []string{ - `route add svc /foo http://bar:111/ weight 0`, + { + "dyn weight 2 -> auto distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 1.00`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.5000`, + `route add svc /foo http://bar:222/ weight 0.5000`, + } }, - []int{100}, }, - { // only fixed weights and sum(fixedWeight) < 1 -> normalize to 100% - []string{ - `route add svc /foo http://bar:111/ weight 0.2`, - `route add svc /foo http://bar:222/ weight 0.3`, + { + "dyn weight 3 -> auto distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/`, + `route add svc /foo http://bar:333/`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.40`, - `route add svc /foo http://bar:222/ weight 0.60`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.3333`, + `route add svc /foo http://bar:222/ weight 0.3333`, + `route add svc /foo http://bar:333/ weight 0.3333`, + } }, - []int{40, 60}, }, - { // only fixed weights and sum(fixedWeight) > 1 -> normalize to 100% - []string{ - `route add svc /foo http://bar:111/ weight 2`, - `route add svc /foo http://bar:222/ weight 3`, + { + "fixed weight 0 -> auto distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.40`, - `route add svc /foo http://bar:222/ weight 0.60`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 1.0000`, + } }, - []int{40, 60}, }, - // TODO(fs): should Table de-duplicate? - { // multiple entries with no fixed weight -> even distribution (same service) - []string{ - `route add svc /foo http://bar:111/`, - `route add svc /foo http://bar:111/`, + { + "only fixed weights and sum(fixedWeight) < 1 -> normalize to 100%", + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.2`, + `route add svc /foo http://bar:222/ weight 0.3`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.50`, - `route add svc /foo http://bar:111/ weight 0.50`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.4000`, + `route add svc /foo http://bar:222/ weight 0.6000`, + } }, - []int{100, 100}, }, - { // multiple entries with no fixed weight -> even distribution - []string{ - `route add svc /foo http://bar:111/`, - `route add svc /foo http://bar:222/`, + { + "only fixed weights and sum(fixedWeight) > 1 -> normalize to 100%", + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 2`, + `route add svc /foo http://bar:222/ weight 3`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.50`, - `route add svc /foo http://bar:222/ weight 0.50`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.4000`, + `route add svc /foo http://bar:222/ weight 0.6000`, + } }, - []int{50, 50}, }, - { // mixed fixed and auto weights -> even distribution of remaining weight across non-fixed weighted targets - []string{ - `route add svc /foo http://bar:111/`, - `route add svc /foo http://bar:222/`, - `route add svc /foo http://bar:333/ weight 0.5`, + { + "multiple entries for same instance with no fixed weight -> de-duplication", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:111/`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.25`, - `route add svc /foo http://bar:222/ weight 0.25`, - `route add svc /foo http://bar:333/ weight 0.50`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 1.0000`, + } }, - []int{25, 25, 50}, }, - { // fixed weight == 100% -> route only to fixed weighted targets - []string{ - `route add svc /foo http://bar:111/`, - `route add svc /foo http://bar:222/ weight 0.25`, - `route add svc /foo http://bar:333/ weight 0.75`, + { + "multiple entries with no fixed weight -> even distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/`, + } }, - []string{ - `route add svc /foo http://bar:222/ weight 0.25`, - `route add svc /foo http://bar:333/ weight 0.75`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.5000`, + `route add svc /foo http://bar:222/ weight 0.5000`, + } }, - []int{0, 25, 75}, }, - { // fixed weight > 100% -> route only to fixed weighted targets and normalize weight - []string{ - `route add svc /foo http://bar:111/`, - `route add svc /foo http://bar:222/ weight 1`, - `route add svc /foo http://bar:333/ weight 3`, + { + "multiple entries with de-dup and no fixed weight -> even distribution", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/`, + } }, - []string{ - `route add svc /foo http://bar:222/ weight 0.25`, - `route add svc /foo http://bar:333/ weight 0.75`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.5000`, + `route add svc /foo http://bar:222/ weight 0.5000`, + } }, - []int{0, 25, 75}, }, - { // dynamic weight matched on service name - []string{ - `route add svca /foo http://bar:111/`, - `route add svcb /foo http://bar:222/`, - `route add svcb /foo http://bar:333/`, - `route weight svcb /foo weight 0.1`, + { + "mixed fixed and auto weights -> even distribution of remaining weight across non-fixed weighted targets", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/`, + `route add svc /foo http://bar:333/ weight 0.5`, + } }, - []string{ - `route add svca /foo http://bar:111/ weight 0.90`, - `route add svcb /foo http://bar:222/ weight 0.05`, - `route add svcb /foo http://bar:333/ weight 0.05`, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.2500`, + `route add svc /foo http://bar:222/ weight 0.2500`, + `route add svc /foo http://bar:333/ weight 0.5000`, + } }, - []int{90, 5, 5}, }, - { // dynamic weight matched on service name and tags - []string{ - `route add svc /foo http://bar:111/ tags "a"`, - `route add svc /foo http://bar:222/ tags "b"`, - `route add svc /foo http://bar:333/ tags "b"`, - `route weight svc /foo weight 0.1 tags "b"`, + { + "fixed weight == 100% -> route only to fixed weighted targets", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/ weight 0.2500`, + `route add svc /foo http://bar:333/ weight 0.7500`, + } }, - []string{ - `route add svc /foo http://bar:111/ weight 0.90 tags "a"`, - `route add svc /foo http://bar:222/ weight 0.05 tags "b"`, - `route add svc /foo http://bar:333/ weight 0.05 tags "b"`, + func() []string { + return []string{ + `route add svc /foo http://bar:222/ weight 0.2500`, + `route add svc /foo http://bar:333/ weight 0.7500`, + } }, - []int{90, 5, 5}, }, - { // dynamic weight matched on tags - []string{ - `route add svca /foo http://bar:111/ tags "a"`, - `route add svcb /foo http://bar:222/ tags "b"`, - `route add svcb /foo http://bar:333/ tags "b"`, - `route weight /foo weight 0.1 tags "b"`, + { + "fixed weight > 100% -> route only to fixed weighted targets and normalize weight", + func() []string { + return []string{ + `route add svc /foo http://bar:111/`, + `route add svc /foo http://bar:222/ weight 1`, + `route add svc /foo http://bar:333/ weight 3`, + } }, - []string{ - `route add svca /foo http://bar:111/ weight 0.90 tags "a"`, - `route add svcb /foo http://bar:222/ weight 0.05 tags "b"`, - `route add svcb /foo http://bar:333/ weight 0.05 tags "b"`, + func() []string { + return []string{ + `route add svc /foo http://bar:222/ weight 0.2500`, + `route add svc /foo http://bar:333/ weight 0.7500`, + } + }, + }, + + { + "dynamic weight matched on service name", + func() []string { + return []string{ + `route add svca /foo http://bar:111/`, + `route add svcb /foo http://bar:222/`, + `route add svcb /foo http://bar:333/`, + `route weight svcb /foo weight 0.1`, + } + }, + func() []string { + return []string{ + `route add svca /foo http://bar:111/ weight 0.9000`, + `route add svcb /foo http://bar:222/ weight 0.0500`, + `route add svcb /foo http://bar:333/ weight 0.0500`, + } + }, + }, + + { + "dynamic weight matched on service name and tags", + func() []string { + return []string{ + `route add svc /foo http://bar:111/ tags "a"`, + `route add svc /foo http://bar:222/ tags "b"`, + `route add svc /foo http://bar:333/ tags "b"`, + `route weight svc /foo weight 0.1 tags "b"`, + } + }, + func() []string { + return []string{ + `route add svc /foo http://bar:111/ weight 0.9000 tags "a"`, + `route add svc /foo http://bar:222/ weight 0.0500 tags "b"`, + `route add svc /foo http://bar:333/ weight 0.0500 tags "b"`, + } + }, + }, + + { + "dynamic weight matched on tags", + func() []string { + return []string{ + `route add svca /foo http://bar:111/ tags "a"`, + `route add svcb /foo http://bar:222/ tags "b"`, + `route add svcb /foo http://bar:333/ tags "b"`, + `route weight /foo weight 0.1 tags "b"`, + } + }, + func() []string { + return []string{ + `route add svca /foo http://bar:111/ weight 0.9000 tags "a"`, + `route add svcb /foo http://bar:222/ weight 0.0500 tags "b"`, + `route add svcb /foo http://bar:333/ weight 0.0500 tags "b"`, + } + }, + }, + + { + "more than 1000 routes", + func() (a []string) { + for i := 0; i < 2504; i++ { + a = append(a, fmt.Sprintf(`route add svc /foo http://bar:%d/`, i)) + } + return a + }, + func() (a []string) { + for i := 0; i < 2504; i++ { + a = append(a, fmt.Sprintf(`route add svc /foo http://bar:%d/ weight 0.0004`, i)) + } + return a + }, + }, + + { + "more than 1000 routes with a fixed route target", + func() (a []string) { + for i := 0; i < 2504; i++ { + a = append(a, fmt.Sprintf(`route add svc /foo http://bar:%d/`, i)) + } + a = append(a, `route add svc /foo http://static:12345/ tags "a"`) + a = append(a, `route weight svc /foo weight 0.2 tags "a"`) + return a + }, + func() (a []string) { + for i := 0; i < 2504; i++ { + a = append(a, fmt.Sprintf(`route add svc /foo http://bar:%d/ weight 0.0003`, i)) + } + a = append(a, `route add svc /foo http://static:12345/ weight 0.2000 tags "a"`) + return a }, - []int{90, 5, 5}, }, } - for i, tt := range tests { - tbl, err := ParseString(strings.Join(tt.in, "\n")) + atof := func(s string) float64 { + n, err := strconv.ParseFloat(s, 64) if err != nil { - t.Fatalf("%d: got %v want nil", i, err) - } - if got, want := tbl.Config(true), tt.out; !reflect.DeepEqual(got, want) { - t.Errorf("%d: got\n%s\nwant\n%s", i, strings.Join(got, "\n"), strings.Join(want, "\n")) + panic(err) } + return n + } - // count url occurrences - r := tbl.route("", "/foo") - if r == nil { - t.Fatalf("%d: got nil want route /foo", i) - } - for j, s := range tt.in { - if !strings.HasPrefix(s, "route add") { - continue + for _, tt := range tests { + tt := tt // capture loop var + t.Run(tt.desc, func(t *testing.T) { + in, out := tt.in(), tt.out() + + // parse the routes + start := time.Now() + tbl, err := ParseString(strings.Join(in, "\n")) + if err != nil { + t.Fatalf("got %v want nil", err) } - p := strings.Split(s, " ") - if got, want := r.targetWeight(p[4]), tt.counts[j]; got != want { - t.Errorf("%d: %s: got %d want %d", i, p[4], got, want) + t.Logf("parsing %d routes took %s seconds\n", len(in), time.Since(start)) + + // compare the generated routes with the normalized weights + if got, want := tbl.Config(true), out; !reflect.DeepEqual(got, want) { + t.Errorf("got\n%s\nwant\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) } - } + + // fetch the route + r := tbl.route("", "/foo") + if r == nil { + t.Fatalf("got nil want route /foo") + } + + // check that there are at least some slots + if len(r.wTargets) == 0 { + t.Fatalf("got 0 targets want some") + } + + // count how often the 'url' from 'route add svc / ' + // appears in the list of wTargets for all the URLs + // from the routes to determine whether the actual + // distribution of each target within the wTarget slice + // matches what we expect + for _, s := range out { + // skip the 'route weight' lines + if !strings.HasPrefix(s, "route add") { + continue + } + + // route add weight ...`, + p := strings.Split(s, " ") + svcurl, count := p[4], 0 + for _, tg := range r.wTargets { + if tg.URL.String() == svcurl { + count++ + } + } + + // calc the weight of the route as nSlots/totalSlots + gotWeight := float64(count) / float64(len(r.wTargets)) + + // round the weight down to the number of decimal points + // supported by maxSlots + gotWeight = float64(int(gotWeight*float64(maxSlots))) / float64(maxSlots) + + // we want the weight as specified in the generated config + wantWeight := atof(p[6]) + + // check that the actual weight is within 2% of the computed weight + if math.Abs(gotWeight-wantWeight) > 0.02 { + t.Errorf("got weight %f want %f", gotWeight, wantWeight) + } + + // TODO(fs): verify distriibution of targets across the ring + } + }) } }