Skip to content

feat: add time to live for every idle connection#793

Closed
liuzhaohui wants to merge 2 commits intoredis:mainfrom
liuzhaohui:feat_add_conn_lifetime
Closed

feat: add time to live for every idle connection#793
liuzhaohui wants to merge 2 commits intoredis:mainfrom
liuzhaohui:feat_add_conn_lifetime

Conversation

@liuzhaohui
Copy link
Copy Markdown
Contributor

Intuition

When ClientOption.BlockingPoolCleanup is set, all connections beyond BlockingPoolMinSize in idle list of the pool will be recycled. Which will cause connection be closed and created frequently even if my application has high concurrency. As my obervation, at the very moment the timer function is triggered, there may be more than BlockingPoolMinSize of connection in the idle list which are store recently and supposed to be acquired very soon.

Approach

My immediate thougth is adding an access time to every connection. The access time is updated when connection is put back into the pool. Access time will be checked when recycling. If time.Since(accessTime) < BlockingPoolConnIdleTime, that connection will not be recycled. BlockingPoolConnIdleTime is a new ClientOption to define the idle time for every connection.

@rueian
Copy link
Copy Markdown
Collaborator

rueian commented Mar 4, 2025

Hi @liuzhaohui, thank you for the contribution, but I don’t think we really need this, and storing access time looks expensive. You can avoid frequent reconnections by a larger BlockingPoolMinSize and a longer BlockingPoolCleanup.

@liuzhaohui
Copy link
Copy Markdown
Contributor Author

Hi @liuzhaohui, thank you for the contribution, but I don’t think we really need this, and storing access time looks expensive. You can avoid frequent reconnections by a larger BlockingPoolMinSize and a longer BlockingPoolCleanup.

Thanks for replying. Currently, I'm using a large BlockingPoolMinSize and setting BlockingPoolCleanup to 0 to disable clean up. As you mentioned, this is not a necessary feature.

@liuzhaohui liuzhaohui closed this Mar 4, 2025
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.

2 participants