Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Redis client configuration logic #104

Merged
merged 30 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
731bfbb
Implement Redis client configuration logic
aaron-congo Feb 7, 2024
e4526af
Change default uint32 fields to *uint32
aaron-congo Feb 21, 2024
364386f
Add gofumpt
aaron-congo Feb 21, 2024
111f656
Add struct validation logic
aaron-congo Feb 21, 2024
6dfc602
Format line length
aaron-congo Feb 21, 2024
5cf5784
Format with fumpt
aaron-congo Feb 21, 2024
9f76bad
Move build tool versions to makefile
aaron-congo Feb 21, 2024
28ead19
Fix makefile lint/format targets
aaron-congo Feb 21, 2024
59c53e6
Download old version of staticcheck if using go 1.18
aaron-congo Feb 21, 2024
6cd71e4
Uncomment install commands
aaron-congo Feb 21, 2024
2143942
Add make target to install go1.18 tools
aaron-congo Feb 22, 2024
dfe3e32
Fix incorrect version number
aaron-congo Feb 22, 2024
39177c2
Add golines to tool installations
aaron-congo Feb 22, 2024
e5ed300
Add debug step to GH actions
aaron-congo Feb 22, 2024
43d9a13
Remove goimports as a tool dependency
aaron-congo Feb 22, 2024
7f5059d
PR suggestions
aaron-congo Feb 22, 2024
e223baf
Remove typo
aaron-congo Feb 22, 2024
6ae23a1
Improve makefile targets and target names
aaron-congo Feb 22, 2024
8f12115
Fix incorrect dev tool versions in makefile
aaron-congo Feb 22, 2024
8b8971b
Change editorconfig defaults for Go
aaron-congo Feb 22, 2024
81c58af
Update CI to run against Go 1.22
aaron-congo Feb 22, 2024
0fab94a
Add DEVELOPER.md
aaron-congo Feb 22, 2024
7eb12fd
PR suggestions
aaron-congo Feb 22, 2024
810791f
Change struct fields to no longer be pointers
aaron-congo Feb 27, 2024
ae473a8
PR suggestions
aaron-congo Feb 27, 2024
d046752
Fix makefile target name
aaron-congo Feb 27, 2024
6301ca6
PR suggestions
aaron-congo Feb 27, 2024
a62b9fe
PR suggestions
aaron-congo Feb 27, 2024
50286f1
PR suggestions
aaron-congo Feb 28, 2024
3422c91
Add clientName field
aaron-congo Feb 29, 2024
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
14 changes: 13 additions & 1 deletion go/Makefile

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using linter for makefile too (later)

Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
install-tools:
@cat tools.go | grep _ | awk -F'"' '{print $$2}' | xargs -tI % go install %
go install github.com/vakenbolt/[email protected]
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
go install google.golang.org/protobuf/cmd/[email protected]
go install honnef.co/go/tools/cmd/[email protected]
go install mvdan.cc/[email protected]
go install golang.org/x/tools/cmd/[email protected]

build: build-glide-core build-glide-client generate-protobuf
go build ./...
Expand All @@ -23,6 +27,14 @@ generate-protobuf:
lint:
go vet ./...
staticcheck ./...
if [ "$$(gofumpt -l . | wc -l)" -gt 0 ]; then exit 1; fi
if [ "$$(golines -l --shorten-comments -m 127 . | wc -l)" -gt 0 ]; then exit 1; fi
if [ "$$(goimports -l . | wc -l)" -gt 0 ]; then exit 1; fi

format:
gofumpt -w .
golines -w --shorten-comments -m 127 .
goimports -w .

unit-test-report:
mkdir -p reports
Expand Down
315 changes: 315 additions & 0 deletions go/api/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,315 @@
/**
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/

package api

import "github.com/aws/glide-for-redis/go/glide/protobuf"

// NodeAddress represents the host address and port of a node in the cluster.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently go doc comments are supposed to include the name of the type/func in the first sentence, so the docs are slightly different than the other clients to accommodate that. I've tried to keep them as close possible still though

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yury-Fridlyand present tense will be forced for Go it seems ;)

type NodeAddress struct {
Host *string // Optional: if not supplied, "localhost" will be used.
Port *uint32 // Optional: if not supplied, 6379 will be used.
}

// RedisCredentials represents the credentials for connecting to a Redis server.
type RedisCredentials struct {
// Optional: the username that will be used for authenticating connections to the Redis servers. If not supplied, "default"
// will be used.
Username *string
// Required: the password that will be used for authenticating connections to the Redis servers.
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
Password *string
}

func (credentials *RedisCredentials) validate() error {
if credentials.Password == nil {
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
return &RedisError{
"RedisCredentials.Password evaluated to nil. Password must be non-nil to authenticate with credentials.",
}
}

return nil
}

// ReadFrom represents the client's read from strategy.
type ReadFrom int

const (
// Primary - Always get from primary, in order to get the freshest data.
Primary ReadFrom = 0
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
// PreferReplica - Spread the requests between all replicas in a round-robin manner. If no replica is available, route the
// requests to the primary.
PreferReplica ReadFrom = 1
)

type baseClientConfiguration struct {
addresses []NodeAddress
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made these fields private (indicated by the lower-case first letter) so that the user has to use the With* builder methods instead of accessing them directly

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of putting docs for these private fields here, I've put the docs above the associated With* builder methods below

useTLS bool
credentials *RedisCredentials
readFrom ReadFrom
requestTimeout *uint32
}

func (config *baseClientConfiguration) toProtobufConnRequest() (*protobuf.ConnectionRequest, error) {
request := protobuf.ConnectionRequest{}
for _, address := range config.addresses {
if address.Host == nil {
defaultHost := "localhost"
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
address.Host = &defaultHost
}

if address.Port == nil {
defaultPort := uint32(6379)
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
address.Port = &defaultPort
}

nodeAddress := &protobuf.NodeAddress{
Host: *address.Host,
Port: *address.Port,
}
request.Addresses = append(request.Addresses, nodeAddress)
}

if config.useTLS {
request.TlsMode = protobuf.TlsMode_SecureTls
} else {
request.TlsMode = protobuf.TlsMode_NoTls
}

if config.credentials != nil {
if err := config.credentials.validate(); err != nil {
return nil, err
}

authInfo := protobuf.AuthenticationInfo{}
if config.credentials.Username != nil {
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
authInfo.Username = *config.credentials.Username
}
authInfo.Password = *config.credentials.Password
request.AuthenticationInfo = &authInfo
}

request.ReadFrom = mapReadFrom(config.readFrom)
if config.requestTimeout != nil {
request.RequestTimeout = *config.requestTimeout
}

return &request, nil
}

func mapReadFrom(readFrom ReadFrom) protobuf.ReadFrom {
if readFrom == PreferReplica {
return protobuf.ReadFrom_PreferReplica
}

return protobuf.ReadFrom_Primary
}

// BackoffStrategy represents the strategy used to determine how and when to reconnect, in case of connection failures. The
// time between attempts grows exponentially, to the formula rand(0 ... factor * (exponentBase ^ N)), where N is the number of
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
// failed attempts.
//
// Once the maximum value is reached, that will remain the time between retry attempts until a reconnect attempt is successful.
// The client will attempt to reconnect indefinitely.
type BackoffStrategy struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaron-congo can you confirm that the formatting for these comments is preserved when generating documentation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line length is not preserved, but blank lines and indents are. So docs for the above look like this (you can get them by running go doc -all github.com/aws/glide-for-redis/go/glide/api

    BackoffStrategy represents the strategy used to determine how and when to
    reconnect, in case of connection failures. The time between attempts grows
    exponentially, to the formula:

        rand(0 ... factor * (exponentBase ^ N))

    where N is the number of failed attempts.

    Once the maximum value is reached, that will remain the time between retry
    attempts until a reconnect attempt is successful. The client will attempt to
    reconnect indefinitely.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about font for the coded section and "N". Can we use backticks (like Python) or add html tags (like <code> and <pre>)?

Copy link
Author

@aaron-congo aaron-congo Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Go docs you create code blocks by indenting the relevant code, like I've done here. There is no inline code highlighting like you would get with backticks. When you upload the package the site will automatically render the Go docs into html, which includes rendering the indented code blocks as <pre> blocks

// Required: number of retry attempts that the client should perform when disconnected from the server, where the time
// between retries increases. Once the retries have reached the maximum value, the time between retries will remain
// constant until a reconnect attempt is successful.
NumOfRetries *uint32
// Required: the multiplier that will be applied to the waiting time between each retry.
Factor *uint32
// Required: the exponent base configured for the strategy.
ExponentBase *uint32
}

func (strategy *BackoffStrategy) validate() error {
if strategy.NumOfRetries == nil {
return &RedisError{
"BackoffStrategy.NumOfRetries evaluated to nil. NumOfRetries must be non-nil to use a BackoffStrategy.",
}
}

if strategy.Factor == nil {
return &RedisError{
"BackoffStrategy.Factor evaluated to nil. Factor must be non-nil to use a BackoffStrategy.",
}
}

if strategy.ExponentBase == nil {
return &RedisError{
"BackoffStrategy.ExponentBase evaluated to nil. ExponentBase must be non-nil to use a BackoffStrategy.",
}
}

return nil
}

// RedisClientConfiguration represents the configuration settings for a Standalone Redis client. baseClientConfiguration is an
// embedded struct that contains shared settings for standalone and cluster clients.
type RedisClientConfiguration struct {
baseClientConfiguration
reconnectStrategy *BackoffStrategy
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
databaseId *uint32
}

// NewRedisClientConfiguration returns a [RedisClientConfiguration] with default configuration settings. For further
// configuration, use the [RedisClientConfiguration] With* methods.
func NewRedisClientConfiguration() *RedisClientConfiguration {
aaron-congo marked this conversation as resolved.
Show resolved Hide resolved
return &RedisClientConfiguration{}
}

func (config *RedisClientConfiguration) toProtobufConnRequest() (*protobuf.ConnectionRequest, error) {
request, err := config.baseClientConfiguration.toProtobufConnRequest()
if err != nil {
return nil, err
}

request.ClusterModeEnabled = false
if config.reconnectStrategy != nil {
if err = config.reconnectStrategy.validate(); err != nil {
return nil, err
}

request.ConnectionRetryStrategy = &protobuf.ConnectionRetryStrategy{
NumberOfRetries: *config.reconnectStrategy.NumOfRetries,
Factor: *config.reconnectStrategy.Factor,
ExponentBase: *config.reconnectStrategy.ExponentBase,
}
}

if config.databaseId != nil {
request.DatabaseId = *config.databaseId
}

return request, nil
}

// WithAddress adds an address for a known node in the cluster to this configuration's list of addresses. WithAddress can be
// called multiple times to add multiple addresses to the list. If the server is in cluster mode the list can be partial, as
// the client will attempt to map out the cluster and find all nodes. If the server is in standalone mode, only nodes whose
// addresses were provided will be used by the client. For example:
//
// config := NewRedisClientConfiguration().
// WithAddress(&NodeAddress{
// Host: "sample-address-0001.use1.cache.amazonaws.com", Port: 6379}).
// WithAddress(&NodeAddress{
// Host: "sample-address-0002.use1.cache.amazonaws.com", Port: 6379})
func (config *RedisClientConfiguration) WithAddress(address *NodeAddress) *RedisClientConfiguration {
config.addresses = append(config.addresses, *address)
return config
}

// WithUseTLS configures the TLS settings for this configuration. Set to true if communication with the cluster should use
// Transport Level Security. This setting should match the TLS configuration of the server/cluster, otherwise the connection
// attempt will fail.
func (config *RedisClientConfiguration) WithUseTLS(useTLS bool) *RedisClientConfiguration {
config.useTLS = useTLS
return config
}

// WithCredentials sets the credentials for the authentication process. If none are set, the client will not authenticate
// itself with the server.
func (config *RedisClientConfiguration) WithCredentials(credentials *RedisCredentials) *RedisClientConfiguration {
config.credentials = credentials
return config
}

// WithReadFrom sets the client's [ReadFrom] strategy. If not set, [Primary] will be used.
func (config *RedisClientConfiguration) WithReadFrom(readFrom ReadFrom) *RedisClientConfiguration {
config.readFrom = readFrom
return config
}

// WithRequestTimeout sets the duration in milliseconds that the client should wait for a request to complete. This duration
// encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the
// specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be
// used.
func (config *RedisClientConfiguration) WithRequestTimeout(requestTimeout uint32) *RedisClientConfiguration {
config.requestTimeout = &requestTimeout
return config
}

// WithReconnectStrategy sets the [BackoffStrategy] used to determine how and when to reconnect, in case of connection
// failures. If not set, a default backoff strategy will be used.
func (config *RedisClientConfiguration) WithReconnectStrategy(strategy *BackoffStrategy) *RedisClientConfiguration {
config.reconnectStrategy = strategy
return config
}

// WithDatabaseId sets the index of the logical database to connect to.
func (config *RedisClientConfiguration) WithDatabaseId(id uint32) *RedisClientConfiguration {
config.databaseId = &id
return config
}

// RedisClusterClientConfiguration represents the configuration settings for a Cluster Redis client.
// Note: Currently, the reconnection strategy in cluster mode is not configurable, and exponential backoff with fixed values is
// used.
type RedisClusterClientConfiguration struct {
baseClientConfiguration
}

// NewRedisClusterClientConfiguration returns a [RedisClientConfiguration] with default configuration settings. For further
// configuration, use the [RedisClientConfiguration] With* methods.
func NewRedisClusterClientConfiguration() *RedisClusterClientConfiguration {
return &RedisClusterClientConfiguration{
baseClientConfiguration: baseClientConfiguration{},
}
}

func (config *RedisClusterClientConfiguration) toProtobufConnRequest() (*protobuf.ConnectionRequest, error) {
request, err := config.baseClientConfiguration.toProtobufConnRequest()
if err != nil {
return nil, err
}

request.ClusterModeEnabled = true
return request, nil
}

// WithAddress adds an address for a known node in the cluster to this configuration's list of addresses. WithAddress can be
// called multiple times to add multiple addresses to the list. If the server is in cluster mode the list can be partial, as
// the client will attempt to map out the cluster and find all nodes. If the server is in standalone mode, only nodes whose
// addresses were provided will be used by the client. For example:
//
// config := NewRedisClusterClientConfiguration().
// WithAddress(&NodeAddress{
// Host: "sample-address-0001.use1.cache.amazonaws.com", Port: 6379}).
// WithAddress(&NodeAddress{
// Host: "sample-address-0002.use1.cache.amazonaws.com", Port: 6379})
func (config *RedisClusterClientConfiguration) WithAddress(address NodeAddress) *RedisClusterClientConfiguration {
config.addresses = append(config.addresses, address)
return config
}

// WithUseTLS configures the TLS settings for this configuration. Set to true if communication with the cluster should use
// Transport Level Security. This setting should match the TLS configuration of the server/cluster, otherwise the connection
// attempt will fail.
func (config *RedisClusterClientConfiguration) WithUseTLS(useTLS bool) *RedisClusterClientConfiguration {
config.useTLS = useTLS
return config
}

// WithCredentials sets the credentials for the authentication process. If none are set, the client will not authenticate
// itself with the server.
func (config *RedisClusterClientConfiguration) WithCredentials(
credentials *RedisCredentials,
) *RedisClusterClientConfiguration {
config.credentials = credentials
return config
}

// WithReadFrom sets the client's [ReadFrom] strategy. If not set, [Primary] will be used.
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
func (config *RedisClusterClientConfiguration) WithReadFrom(readFrom ReadFrom) *RedisClusterClientConfiguration {
config.readFrom = readFrom
return config
}

// WithRequestTimeout sets the duration in milliseconds that the client should wait for a request to complete. This duration
// encompasses sending the request, awaiting for a response from the server, and any required reconnections or retries. If the
// specified timeout is exceeded for a pending request, it will result in a timeout error. If not set, a default value will be
// used.
func (config *RedisClusterClientConfiguration) WithRequestTimeout(requestTimeout uint32) *RedisClusterClientConfiguration {
config.requestTimeout = &requestTimeout
return config
}
Loading
Loading