From 0915002c686bf5a4b2fd12ac37827a94ddff76e6 Mon Sep 17 00:00:00 2001 From: Andrew de Andrade Date: Wed, 26 Jul 2017 13:14:09 -0700 Subject: [PATCH 1/4] Implement router.GetNClients --- router/router.go | 32 ++++++++++++++++++++++++++++++++ router/router_test.go | 31 ++++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/router/router.go b/router/router.go index 148a64be..54ed72d0 100644 --- a/router/router.go +++ b/router/router.go @@ -46,6 +46,11 @@ type Router interface { // time it will inform the caller if the client is a remote client or a // local client via the isRemote return value. GetClient(key string) (client interface{}, isRemote bool, err error) + // GetNClients provides the caller with an ordered slice of clients for a + // given key. Each result is a struct with a reference to the actual client + // and a bool indicating whether or not that client is a remote client or a + // local client. + GetNClients(key string, n int) (clients []ClientResult, err error) } // A ClientFactory is able to provide an implementation of a TChan[Service] @@ -101,6 +106,33 @@ func (r *router) GetClient(key string) (client interface{}, isRemote bool, err e return nil, false, err } + return r.getClientByHost(dest) +} + +// ClientResult is a struct that contains a reference to the actual callable +// client and a bool indicating whether or not that client is local or remote. +type ClientResult struct { + Client interface{} + IsRemote bool +} + +func (r *router) GetNClients(key string, n int) (clients []ClientResult, err error) { + dests, err := r.ringpop.LookupN(key, n) + if err != nil { + return nil, err + } + + for _, dest := range dests { + client, isRemote, err := r.getClientByHost(dest) + if err != nil { + return nil, err + } + clients = append(clients, ClientResult{client, isRemote}) + } + return clients, nil +} + +func (r *router) getClientByHost(dest string) (client interface{}, isRemote bool, err error) { r.rw.RLock() cachedEntry, ok := r.clientCache[dest] r.rw.RUnlock() diff --git a/router/router_test.go b/router/router_test.go index 00fb6c42..23f2f69a 100644 --- a/router/router_test.go +++ b/router/router_test.go @@ -52,6 +52,9 @@ func (s *RouterTestSuite) SetupTest() { s.ringpop.On("Lookup", "remote2").Return("127.0.0.1:3001", nil) s.ringpop.On("Lookup", "error").Return("", errors.New("ringpop not ready")) + s.ringpop.On("LookupN", "localfirst", 2).Return([]string{"127.0.0.1:3000", "127.0.0.1:3001"}, nil) + s.ringpop.On("LookupN", "remotefirst", 2).Return([]string{"127.0.0.1:3001", "127.0.0.1:3000"}, nil) + ch, err := tchannel.NewChannel("remote", nil) s.NoError(err) @@ -67,6 +70,28 @@ func (s *RouterTestSuite) TestRingpopRouterGetLocalClient() { s.clientFactory.AssertCalled(s.T(), "GetLocalClient") } +func (s *RouterTestSuite) TestRingpopRouterGetNClientsLocalFirst() { + clients, err := s.router.GetNClients("localfirst", 2) + s.NoError(err) + + s.Equal("local client", clients[0].Client) + s.Equal("remote client", clients[1].Client) + + s.False(clients[0].IsRemote, "first client for localfirst key should not be a remote client") + s.True(clients[1].IsRemote, "second client for localfirst key should not be a local client") +} + +func (s *RouterTestSuite) TestRingpopRouterGetNClientsRemoteFirst() { + clients, err := s.router.GetNClients("remotefirst", 2) + s.NoError(err) + + s.Equal("remote client", clients[0].Client) + s.Equal("local client", clients[1].Client) + + s.True(clients[0].IsRemote, "first client for localfirst key should not be a local client") + s.False(clients[1].IsRemote, "second client for localfirst key should not be a remote client") +} + func (s *RouterTestSuite) TestRingpopRouterGetLocalClientCached() { client, isRemote, err := s.router.GetClient("local") s.NoError(err) @@ -168,7 +193,7 @@ func (s *RouterTestSuite) TestRingpopRouterRemoveClientOnSwimFaultyEvent() { s.NoError(err) s.internal.HandleEvent(swim.MemberlistChangesReceivedEvent{ Changes: []swim.Change{ - swim.Change{ + { Address: dest, Status: swim.Faulty, }, @@ -191,7 +216,7 @@ func (s *RouterTestSuite) TestRingpopRouterRemoveClientOnSwimLeaveEvent() { s.NoError(err) s.internal.HandleEvent(swim.MemberlistChangesReceivedEvent{ Changes: []swim.Change{ - swim.Change{ + { Address: dest, Status: swim.Leave, }, @@ -214,7 +239,7 @@ func (s *RouterTestSuite) TestRingpopRouterNotRemoveClientOnSwimSuspectEvent() { s.NoError(err) s.internal.HandleEvent(swim.MemberlistChangesReceivedEvent{ Changes: []swim.Change{ - swim.Change{ + { Address: dest, Status: swim.Suspect, }, From 754ea939282a5514bf35b662182f7663910e4e2c Mon Sep 17 00:00:00 2001 From: Andrew de Andrade Date: Thu, 27 Jul 2017 10:46:13 -0700 Subject: [PATCH 2/4] Pre-allocate slice in GetNClients --- router/router.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/router/router.go b/router/router.go index 54ed72d0..3f5b656d 100644 --- a/router/router.go +++ b/router/router.go @@ -116,18 +116,20 @@ type ClientResult struct { IsRemote bool } -func (r *router) GetNClients(key string, n int) (clients []ClientResult, err error) { +func (r *router) GetNClients(key string, n int) ([]ClientResult, error) { dests, err := r.ringpop.LookupN(key, n) if err != nil { return nil, err } - for _, dest := range dests { + clients make([]ClientResult, 0, n) + + for i, dest := range dests { client, isRemote, err := r.getClientByHost(dest) if err != nil { return nil, err } - clients = append(clients, ClientResult{client, isRemote}) + clients[i] = ClientResult{client, isRemote} } return clients, nil } From 3346e034651ec5137cea3a67ba89055291ff74b8 Mon Sep 17 00:00:00 2001 From: Andrew de Andrade Date: Thu, 27 Jul 2017 10:48:48 -0700 Subject: [PATCH 3/4] Fix GetNClients --- router/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/router.go b/router/router.go index 3f5b656d..91818b30 100644 --- a/router/router.go +++ b/router/router.go @@ -122,7 +122,7 @@ func (r *router) GetNClients(key string, n int) ([]ClientResult, error) { return nil, err } - clients make([]ClientResult, 0, n) + clients := make([]ClientResult, n, n) for i, dest := range dests { client, isRemote, err := r.getClientByHost(dest) From b7bc0be988983dbbd2d26a0d1b9dee9d02a1ff06 Mon Sep 17 00:00:00 2001 From: Andrew de Andrade Date: Thu, 27 Jul 2017 10:54:56 -0700 Subject: [PATCH 4/4] Add HostPort to GetNClients results --- router/router.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/router/router.go b/router/router.go index 91818b30..a69c403e 100644 --- a/router/router.go +++ b/router/router.go @@ -112,6 +112,7 @@ func (r *router) GetClient(key string) (client interface{}, isRemote bool, err e // ClientResult is a struct that contains a reference to the actual callable // client and a bool indicating whether or not that client is local or remote. type ClientResult struct { + HostPort string Client interface{} IsRemote bool } @@ -129,7 +130,7 @@ func (r *router) GetNClients(key string, n int) ([]ClientResult, error) { if err != nil { return nil, err } - clients[i] = ClientResult{client, isRemote} + clients[i] = ClientResult{dest, client, isRemote} } return clients, nil }