Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

WIP GRPC rate limiting #533

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix variable naming from attempts/tries to retries
  • Loading branch information
mikemorris committed Apr 5, 2023
commit f4a3d91b36fb8454c1d0019929a8c4beb7f00a7c
10 changes: 5 additions & 5 deletions internal/consul/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type Authenticator struct {

method string
namespace string
tries uint64
retries uint64
backoffInterval time.Duration
}

Expand All @@ -36,13 +36,13 @@ func NewAuthenticator(logger hclog.Logger, consul *api.Client, method, namespace
logger: logger,
method: method,
namespace: namespace,
tries: defaultMaxAttempts,
retries: defaultMaxRetries,
backoffInterval: defaultBackoffInterval,
}
}

func (a *Authenticator) WithTries(tries uint64) *Authenticator {
a.tries = tries
func (a *Authenticator) WithRetries(retries uint64) *Authenticator {
a.retries = retries
return a
}

Expand All @@ -58,7 +58,7 @@ func (a *Authenticator) Authenticate(ctx context.Context, service, bearerToken s
a.logger.Error("error authenticating", "error", err)
}
return err
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(a.backoffInterval), a.tries), ctx))
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(a.backoffInterval), a.retries), ctx))
return token, err
}

Expand Down
22 changes: 11 additions & 11 deletions internal/consul/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestAuthenticate(t *testing.T) {
service string
expectedMeta string
failures uint64
maxAttempts uint64
maxRetries uint64
fail bool
}{{
name: "success-no-namespace",
Expand All @@ -46,25 +46,25 @@ func TestAuthenticate(t *testing.T) {
service: "consul-api-gateway-test",
expectedMeta: "consul-api-gateway-test",
failures: 3,
maxAttempts: 3,
maxRetries: 3,
}, {
name: "retry-failure",
service: "consul-api-gateway-test",
failures: 3,
maxAttempts: 2,
fail: true,
name: "retry-failure",
service: "consul-api-gateway-test",
failures: 3,
maxRetries: 2,
fail: true,
}} {
t.Run(test.name, func(t *testing.T) {
server := runACLServer(t, test.failures)
method := gwTesting.RandomString()
token := gwTesting.RandomString()

maxAttempts := defaultMaxAttempts
if test.maxAttempts > 0 {
maxAttempts = test.maxAttempts
maxRetries := defaultMaxRetries
if test.maxRetries > 0 {
maxRetries = test.maxRetries
}

auth := NewAuthenticator(hclog.NewNullLogger(), server.consul, method, test.namespace).WithTries(maxAttempts)
auth := NewAuthenticator(hclog.NewNullLogger(), server.consul, method, test.namespace).WithRetries(maxRetries)
auth.backoffInterval = 0
consulToken, err := auth.Authenticate(context.Background(), test.service, token)

Expand Down
2 changes: 1 addition & 1 deletion internal/consul/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ import (
)

const (
defaultMaxAttempts = uint64(30)
defaultMaxRetries = uint64(30)
defaultBackoffInterval = 1 * time.Second
)
39 changes: 20 additions & 19 deletions internal/consul/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type ServiceRegistry struct {
tags []string

cancel context.CancelFunc
tries uint64
retries uint64
backoffInterval time.Duration
reregistrationInterval time.Duration
updateTTLInterval time.Duration
Expand All @@ -54,16 +54,16 @@ func NewServiceRegistryWithAddress(logger hclog.Logger, client Client, service,

// NewServiceRegistry creates a new service registry instance
func NewServiceRegistry(logger hclog.Logger, client Client, service, namespace, partition, host string) *ServiceRegistry {
// TODO: this is probably wrong, should this be the consul-http-addr flag value
address := ""
//TODO this is probably wrong, should this be the consul-http-addr flag value
nodes, _, err := client.Catalog().Nodes(nil)
if err != nil {
address = ""
} else {
for _, n := range nodes {
address = n.Address
}
}

// FIXME: this setup call is currently tracked against the max retries allowed
// by the mock Consul server
// nodes, _, err := client.Catalog().Nodes(nil)
// for _, n := range nodes {
// address = n.Address
// }

return newServiceRegistry(logger, client, service, namespace, partition, host, address)
}

Expand All @@ -77,7 +77,7 @@ func newServiceRegistry(logger hclog.Logger, client Client, service, namespace,
namespace: namespace,
partition: partition,
host: host,
tries: defaultMaxAttempts,
retries: defaultMaxRetries,
backoffInterval: defaultBackoffInterval,
reregistrationInterval: 30 * time.Second,
updateTTLInterval: 10 * time.Second,
Expand All @@ -91,9 +91,9 @@ func (s *ServiceRegistry) WithTags(tags []string) *ServiceRegistry {
return s
}

// WithTries tells the service registry to retry on any remote operations.
func (s *ServiceRegistry) WithTries(tries uint64) *ServiceRegistry {
s.tries = tries
// WithRetries tells the service registry to retry on any remote operations.
func (s *ServiceRegistry) WithRetries(retries uint64) *ServiceRegistry {
s.retries = retries
return s
}

Expand Down Expand Up @@ -212,9 +212,12 @@ func (s *ServiceRegistry) register(ctx context.Context, registration *api.Catalo
}

func (s *ServiceRegistry) ensureRegistration(ctx context.Context, registration *api.CatalogRegistration) {
_, _, err := s.client.Agent().Service(s.id, &api.QueryOptions{
// TODO: will this actually return an error for the catalog API, or just an
// empty list?
_, _, err := s.client.Catalog().Service(s.id, "", &api.QueryOptions{
Namespace: s.namespace,
})

if err == nil {
return
}
Expand Down Expand Up @@ -242,16 +245,14 @@ func (s *ServiceRegistry) retryRegistration(ctx context.Context, registration *a
s.logger.Error("error registering service", "error", err)
}
return err
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(s.backoffInterval), s.tries), ctx))
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(s.backoffInterval), s.retries), ctx))
}

func (s *ServiceRegistry) registerService(ctx context.Context, registration *api.CatalogRegistration) error {

writeOptions := &api.WriteOptions{}
_, err := s.client.Catalog().Register(registration, writeOptions.WithContext(ctx))

return err
//return s.client.Agent().ServiceRegisterOpts(registration, (&api.ServiceRegisterOpts{}).WithContext(ctx))
}

// Deregister de-registers a service from Consul.
Expand All @@ -267,7 +268,7 @@ func (s *ServiceRegistry) Deregister(ctx context.Context) error {
s.logger.Error("error deregistering service", "error", err)
}
return err
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(s.backoffInterval), s.tries), ctx))
}, backoff.WithContext(backoff.WithMaxRetries(backoff.NewConstantBackOff(s.backoffInterval), s.retries), ctx))
}

func (s *ServiceRegistry) deregister(ctx context.Context) error {
Expand Down
66 changes: 33 additions & 33 deletions internal/consul/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,38 +27,38 @@ func TestRegister(t *testing.T) {
ctx := context.Background()

for _, test := range []struct {
name string
host string
failures uint64
maxAttempts uint64
fail bool
name string
host string
failures uint64
maxRetries uint64
fail bool
}{{
name: "basic-test",
host: "localhost",
}, {
name: "test-retries",
host: "localhost",
failures: 3,
maxAttempts: 3,
name: "retry-success",
host: "localhost",
failures: 3,
maxRetries: 3,
}, {
name: "test-retries-fail",
host: "localhost",
failures: 3,
maxAttempts: 2,
fail: true,
name: "retry-failure",
host: "localhost",
failures: 3,
maxRetries: 2,
fail: true,
}} {
t.Run(test.name, func(t *testing.T) {
id := uuid.New().String()
service := gwTesting.RandomString()
namespace := gwTesting.RandomString()

maxAttempts := defaultMaxAttempts
if test.maxAttempts > 0 {
maxAttempts = test.maxAttempts
maxRetries := defaultMaxRetries
if test.maxRetries > 0 {
maxRetries = test.maxRetries
}

server := runRegistryServer(t, test.failures, id)
registry := NewServiceRegistry(hclog.NewNullLogger(), NewTestClient(server.consul), service, namespace, "", test.host).WithTries(maxAttempts)
registry := NewServiceRegistry(hclog.NewNullLogger(), NewTestClient(server.consul), service, namespace, "", test.host).WithRetries(maxRetries)

registry.backoffInterval = 0
registry.id = id
Expand All @@ -85,33 +85,33 @@ func TestRegister(t *testing.T) {
func TestDeregister(t *testing.T) {
t.Parallel()
for _, test := range []struct {
name string
failures uint64
maxAttempts uint64
fail bool
name string
failures uint64
maxRetries uint64
fail bool
}{{
name: "basic-test",
}, {
name: "test-retries",
failures: 3,
maxAttempts: 3,
name: "retry-success",
failures: 3,
maxRetries: 3,
}, {
name: "test-retries-fail",
failures: 3,
maxAttempts: 2,
fail: true,
name: "retry-fail",
failures: 3,
maxRetries: 2,
fail: true,
}} {
t.Run(test.name, func(t *testing.T) {
id := uuid.New().String()
service := gwTesting.RandomString()

maxAttempts := defaultMaxAttempts
if test.maxAttempts > 0 {
maxAttempts = test.maxAttempts
maxRetries := defaultMaxRetries
if test.maxRetries > 0 {
maxRetries = test.maxRetries
}

server := runRegistryServer(t, test.failures, id)
registry := NewServiceRegistry(hclog.NewNullLogger(), NewTestClient(server.consul), service, "", "", "").WithTries(maxAttempts)
registry := NewServiceRegistry(hclog.NewNullLogger(), NewTestClient(server.consul), service, "", "", "").WithRetries(maxRetries)
registry.backoffInterval = 0
registry.id = id
err := registry.Deregister(context.Background())
Expand Down