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

connection pool size #2439

Closed
mshariat7 opened this issue Feb 13, 2023 · 4 comments · Fixed by #2441
Closed

connection pool size #2439

mshariat7 opened this issue Feb 13, 2023 · 4 comments · Fixed by #2441

Comments

@mshariat7
Copy link

Hi
I am using redis.Client in my project with this configs.
poolsize: 10
minIdleConn: 5
maxRetries: 5

the used command is LPUSH for a list of 2000 elements.

var dialCount uint64
client := redis.NewClient(&redis.Options{
	Addr:       "localhost:6379",
	DB:         0,
	MaxRetries: 5,
	Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) {
		fmt.Printf("%d - dialer called for new connection\n", atomic.AddUint64(&dialCount, 1))
		return net.Dial(network, addr)
	},
	PoolSize:     50,
	MinIdleConns: 10,
})

ctx := context.Background()
var items []int
for i := 1; i < 2000; i++ {
	items = append(items, i)
}
pipe := client.Pipeline()
defer func(pipe redis.Pipeliner) {
	err := pipe.Close()
	if err != nil {
		fmt.Println("error in closing pipe")
	}
}(pipe)

key := "test_key"
for _, item := range items {
	cmd := pipe.LPush(ctx, key, item)
	if cmd.Err() != nil {
		fmt.Printf("error in list push is: %v\n", cmd.Err())
		return
	}
}
_, err := pipe.Expire(ctx, key, time.Duration(72)*time.Hour).Result()
if err != nil {
	fmt.Printf("error in expiring pipeline is %v\n", err)
	return
}
_, err = pipe.Exec(ctx)
if err != nil {
	fmt.Printf("error in executing pipe is %v\n", err)
	return
}

signal := make(chan bool)
wg := sync.WaitGroup{}

for i := 0; i < 20000; i++ {
	wg.Add(1)
	go func(ctx context.Context, wg *sync.WaitGroup, n int) {
		defer wg.Done()
		<-signal
		cmd := client.RPop(ctx, key)

		if cmd.Err() != nil {
			if cmd.Err().Error() != "redis: connection pool timeout" && cmd.Err() != redis.Nil {
				fmt.Printf("%d - error: %v\n", n, cmd.Err())
			}
		} else {
			fmt.Printf("%d - value: %v\n", n, cmd.Val())
		}
	}(ctx, &wg, i)
}
close(signal)
wg.Wait()

Expected Behavior

the dial function should be called for 50 times, which is the size of poolSize.

Current Behavior

I see more than 50 connections opens. why client opens connections more than poolSize?

@monkey92t
Copy link
Collaborator

Yes, this phenomenon in the go-redis connection pool is caused by MinIdleConns.

We always hope that there are idle connections in the connection pool that can be used, so we added MinIdleConns. When we find that the number of idle connections in the connection pool is insufficient, goroutine will be started to create idle connections.

If a connection needs to be obtained from the connection pool at this time, we will create a connection immediately instead of waiting for an idle connection.

So, in extreme cases, we create PoolSize + MinIdleConns number of connections.

@monkey92t
Copy link
Collaborator

I will send a PR to fix this issue 😄

@mshariat7
Copy link
Author

Thanks a bunch for your answer and quick fix.

@monkey92t
Copy link
Collaborator

monkey92t commented Feb 14, 2023

fix by #2441

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 a pull request may close this issue.

2 participants