redis / go-redis

Redis Go client
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
20.21k stars 2.38k forks source link

Add Support for Probing Instance for Mode #3107

Open arhea opened 2 months ago

arhea commented 2 months ago

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.

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())
    }
}