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

Add Support for Probing Instance for Mode #3107

Open
arhea opened this issue Sep 6, 2024 · 0 comments
Open

Add Support for Probing Instance for Mode #3107

arhea opened this issue Sep 6, 2024 · 0 comments

Comments

@arhea
Copy link

arhea commented Sep 6, 2024

Expected Behavior

When creating a new NewUniversalClient I expect the proper client Client, ClusterClient, or FailoverClient be created based on the cluster configuration.

Current Behavior

Today, NewUniversalClient uses a simple heuristic for determining which client to create.

  • If MasterName is not an empty string, FailoverClient is created
  • If Addrs slice is greater than 1, create a ClusterClient is created
  • Else create a simple client

This fails when creating the cluster client when only the discovery or primary endpoint is provided. For example, Google Cloud Memory Story only provides a single Discovery endpoint which, will result in a Client being created instead of the ClusterClient.

Possible Solution

This could be enhanced by automatically probing the instance to determine the mode of the instance. Today, we workaround this issue by parsing the connection URL, converting to UniversalOptions, creating a simple client and calling CLUSTER INFO.

universalOpts := &redis.UniversalOptions{}
opts, err := redis.ParseURL(url)

if err != nil {
    return nil, err
}

// convert opts to universalOpts...

var client redis.UniversalClient

tmpClient := redis.NewClient(universalOpts.Simple())
defer tmpClient.Close()

if clusterInfo, err := tmpClient.ClusterInfo(ctx).Result(); err == nil {
    slog.Info("redis is in cluster mode", slog.String("info", clusterInfo))
    client = redis.NewClusterClient(universalOpts.Cluster())
} else {
    slog.Info("redis is not in cluster mode")
    client = redis.NewClient(universalOpts.Simple())
}

Steps to Reproduce

  1. Call NewUniversalClient with a single Addrs entry that is the discovery endpoint for a cluster
  2. A simple Client is Created
  3. Get and Set commands fail with MOVED

Context (Environment)

We leverage Google Cloud and connect to a Redis Cluster. In our test environment, we spin up a single Redis container not in cluster mode. We want the Universal Client to automatically detect the Redis configuration and provision the proper client.

Detailed Description

I suggest adding a new function that will probe the cluster using a simple client and automatically detect the proper client to create. I am not suggesting modifying the existing behavior of NewUniversalClient because this likely works for static clusters or Cloud Providers that provide a list of nodes. In environments, where the cluster configuration can't be determined by a heuristic, a probing function is helpful.

Possible Implementation

I would propose adding a new function called DetectUniversalClient

func DetectUniversalClient(opts *UniversalOptions) UniversalClient {
    if opts.MasterName != "" {
        return NewFailoverClient(opts.Failover())
    }

    tmpClient := redis.NewClient(opts.Simple())
    defer tmpClient.Close()

    if clusterInfo, err := tmpClient.ClusterInfo(ctx).Result(); err == nil {
        return redis.NewClusterClient(opts.Cluster())
    } else {
        return client = redis.NewClient(opts.Simple())
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant