Skip to content

Conversation

@ljluestc
Copy link

Support configurable Redis connection settings

Description

This PR introduces support for configuring MaxRetries, MinIdleConns, DB, and PoolSize in Redis connections via RedisConf.

Previously, these values were hardcoded:

  • MaxRetries: 3
  • MinIdleConns: 8

Changes

  • core/stores/redis/conf.go: Added MaxRetries (default: 3), MinIdleConns (default: 8), DB (default: 0), and PoolSize (optional, defaults to dynamic calculation if 0) to the RedisConf struct.
  • core/stores/redis/redis.go:
    • Updated Redis struct to hold these options.
    • Updated NewRedis and newRedis to accept and apply these options.
    • Added WithMaxRetries, WithMinIdleConns, WithDB, and WithPoolSize options.
  • core/stores/redis/redisclientmanager.go: Updated getClient to initialize the go-redis client with the configured values.
  • core/stores/redis/redisclustermanager.go: Updated getCluster to initialize the go-redis cluster client with the configured values.
  • core/stores/redis/redisblockingnode.go: Updated CreateBlockingNode to respect MaxRetries (while keeping pool size min/max at 1 for blocking operations).

Verification / Test Plan

A new test case TestRedisOptions has been added to core/stores/redis/redis_test.go covering:

  1. Default values (when config is empty/default).
  2. Custom values (when config is provided).
  3. Disabling retries (setting MaxRetries to -1).

To run the specific test:

go test -v ./core/stores/redis/ -run TestRedisOptions

To run all Redis tests:

go test -v ./core/stores/redis/...

Related Issue

Fixes #4668

How to Test

  1. Unit Tests:
    Run the newly added unit tests to verify configuration propagation:

    go test -v core/stores/redis/redis_test.go -run TestRedisOptions
  2. Manual Verification:
    You can verify the configuration by initializing a Redis client with custom values:

    conf := redis.RedisConf{
        Host:         "localhost:6379",
        Type:         redis.NodeType,
        MaxRetries:   5,
        MinIdleConns: 20,
        DB:           1,
        PoolSize:     50,
    }
    r := redis.MustNewRedis(conf)
    // Verify internally or via behavior (e.g. check connection count in redis-cli)

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

Successfully merging this pull request may close these issues.

Why not support Cache use default Redis Conn Configs

1 participant