From b882ff3df9363e11f03246cc6354719d56a812b9 Mon Sep 17 00:00:00 2001 From: ben-toogood Date: Tue, 30 Jun 2020 15:51:26 +0100 Subject: [PATCH] selector: update selector.Select to accept a slice of structs (#1764) --- go.sum | 1 + selector/random.go | 8 +++---- selector/roundrobin/roundrobin.go | 26 +++++++++------------- selector/roundrobin/roundrobin_test.go | 30 +++++++++++++------------- selector/selector.go | 4 ++-- selector/tests.go | 12 +++++------ 6 files changed, 38 insertions(+), 43 deletions(-) diff --git a/go.sum b/go.sum index 8f86e156..f3829ac6 100644 --- a/go.sum +++ b/go.sum @@ -414,6 +414,7 @@ github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a/go.mod h1:s github.com/soheilhy/cmux v0.1.4 h1:0HKaf1o97UwFjHH9o5XsHUOF+tqmdA7KEzXLpiyaw0E= github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/selector/random.go b/selector/random.go index 3821e152..3f384e0c 100644 --- a/selector/random.go +++ b/selector/random.go @@ -16,7 +16,7 @@ func (r *random) Options() Options { return Options{} } -func (r *random) Select(routes []*router.Route) (*router.Route, error) { +func (r *random) Select(routes []router.Route) (*router.Route, error) { // we can't select from an empty pool of routes if len(routes) == 0 { return nil, ErrNoneAvailable @@ -24,14 +24,14 @@ func (r *random) Select(routes []*router.Route) (*router.Route, error) { // if there is only one route provided we'll select it if len(routes) == 1 { - return routes[0], nil + return &routes[0], nil } // select a random route from the slice - return routes[rand.Intn(len(routes)-1)], nil + return &routes[rand.Intn(len(routes)-1)], nil } -func (r *random) Record(route *router.Route, err error) error { +func (r *random) Record(route router.Route, err error) error { return nil } diff --git a/selector/roundrobin/roundrobin.go b/selector/roundrobin/roundrobin.go index 465d19fb..5290ed8e 100644 --- a/selector/roundrobin/roundrobin.go +++ b/selector/roundrobin/roundrobin.go @@ -38,7 +38,7 @@ func (r *roundrobin) Options() selector.Options { return selector.Options{} } -func (r *roundrobin) Select(routes []*router.Route) (*router.Route, error) { +func (r *roundrobin) Select(routes []router.Route) (*router.Route, error) { if len(routes) == 0 { return nil, selector.ErrNoneAvailable } @@ -51,33 +51,27 @@ func (r *roundrobin) Select(routes []*router.Route) (*router.Route, error) { r.routes[hash] = time.Now() } - // calculate the route hashes once - hashes := make(map[*router.Route]uint64, len(routes)) - for _, s := range routes { - hashes[s] = s.Hash() - } - // if a route hasn't yet been seen, prioritise it - for srv, hash := range hashes { - if _, ok := r.routes[hash]; !ok { - setLastUsed(hash) - return srv, nil + for _, route := range routes { + if _, ok := r.routes[route.Hash()]; !ok { + setLastUsed(route.Hash()) + return &route, nil } } // sort the services by the time they were last used sort.SliceStable(routes, func(i, j int) bool { - iLastSeen := r.routes[hashes[routes[i]]] - jLastSeen := r.routes[hashes[routes[j]]] + iLastSeen := r.routes[routes[i].Hash()] + jLastSeen := r.routes[routes[j].Hash()] return iLastSeen.UnixNano() < jLastSeen.UnixNano() }) // return the route which was last used - setLastUsed(hashes[routes[0]]) - return routes[0], nil + setLastUsed(routes[0].Hash()) + return &routes[0], nil } -func (r *roundrobin) Record(srv *router.Route, err error) error { +func (r *roundrobin) Record(srv router.Route, err error) error { return nil } diff --git a/selector/roundrobin/roundrobin_test.go b/selector/roundrobin/roundrobin_test.go index 4062d0d3..c3063a7e 100644 --- a/selector/roundrobin/roundrobin_test.go +++ b/selector/roundrobin/roundrobin_test.go @@ -11,39 +11,39 @@ import ( func TestRoundRobin(t *testing.T) { selector.Tests(t, NewSelector()) - r1 := &router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8000"} - r2 := &router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8001"} - r3 := &router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8002"} + r1 := router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8000"} + r2 := router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8001"} + r3 := router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8002"} sel := NewSelector() // By passing r1 and r2 first, it forces a set sequence of (r1 => r2 => r3 => r1) - r, err := sel.Select([]*router.Route{r1}) + r, err := sel.Select([]router.Route{r1}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r1, r, "Expected route to be r1") + assert.Equal(t, r1, *r, "Expected route to be r1") - r, err = sel.Select([]*router.Route{r2}) + r, err = sel.Select([]router.Route{r2}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r2, r, "Expected route to be r2") + assert.Equal(t, r2, *r, "Expected route to be r2") // Because r1 and r2 have been recently called, r3 should be chosen - r, err = sel.Select([]*router.Route{r1, r2, r3}) + r, err = sel.Select([]router.Route{r1, r2, r3}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r3, r, "Expected route to be r3") + assert.Equal(t, r3, *r, "Expected route to be r3") // r1 was called longest ago, so it should be prioritised - r, err = sel.Select([]*router.Route{r1, r2, r3}) + r, err = sel.Select([]router.Route{r1, r2, r3}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r1, r, "Expected route to be r1") + assert.Equal(t, r1, *r, "Expected route to be r1") - r, err = sel.Select([]*router.Route{r1, r2, r3}) + r, err = sel.Select([]router.Route{r1, r2, r3}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r2, r, "Expected route to be r2") + assert.Equal(t, r2, *r, "Expected route to be r2") - r, err = sel.Select([]*router.Route{r1, r2, r3}) + r, err = sel.Select([]router.Route{r1, r2, r3}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r3, r, "Expected route to be r3") + assert.Equal(t, r3, *r, "Expected route to be r3") } diff --git a/selector/selector.go b/selector/selector.go index 69faf43f..a531eb0f 100644 --- a/selector/selector.go +++ b/selector/selector.go @@ -21,9 +21,9 @@ type Selector interface { // Options the selector is using Options() Options // Select a route from the pool using the strategy - Select([]*router.Route) (*router.Route, error) + Select([]router.Route) (*router.Route, error) // Record the error returned from a route to inform future selection - Record(*router.Route, error) error + Record(router.Route, error) error // Close the selector Close() error // String returns the name of the selector diff --git a/selector/tests.go b/selector/tests.go index bd34e090..e505f5ae 100644 --- a/selector/tests.go +++ b/selector/tests.go @@ -9,24 +9,24 @@ import ( // Tests runs all the tests against a selector to ensure the implementations are consistent func Tests(t *testing.T, s Selector) { - r1 := &router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8000"} - r2 := &router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8001"} + r1 := router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8000"} + r2 := router.Route{Service: "go.micro.service.foo", Address: "127.0.0.1:8001"} t.Run("Select", func(t *testing.T) { t.Run("NoRoutes", func(t *testing.T) { - srv, err := s.Select([]*router.Route{}) + srv, err := s.Select([]router.Route{}) assert.Nil(t, srv, "Route should be nil") assert.Equal(t, ErrNoneAvailable, err, "Expected error to be none available") }) t.Run("OneRoute", func(t *testing.T) { - srv, err := s.Select([]*router.Route{r1}) + srv, err := s.Select([]router.Route{r1}) assert.Nil(t, err, "Error should be nil") - assert.Equal(t, r1, srv, "Expected the route to be returned") + assert.Equal(t, r1, *srv, "Expected the route to be returned") }) t.Run("MultipleRoutes", func(t *testing.T) { - srv, err := s.Select([]*router.Route{r1, r2}) + srv, err := s.Select([]router.Route{r1, r2}) assert.Nil(t, err, "Error should be nil") if srv.Address != r1.Address && srv.Address != r2.Address { t.Errorf("Expected the route to be one of the inputs")