-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(pool): Improve success rate of new connections #3518
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
base: master
Are you sure you want to change the base?
feat(pool): Improve success rate of new connections #3518
Conversation
…l are likely done in async flow
- Fix BenchmarkWantConnQueue_Dequeue timeout issue by limiting pre-population - Use object pooling in BenchmarkWantConnQueue_Enqueue to reduce allocations - Optimize BenchmarkWantConnQueue_EnqueueDequeue with reusable wantConn pool - Prevent GitHub Actions benchmark failures due to excessive memory usage Before: BenchmarkWantConnQueue_Dequeue ran for 11+ minutes and was killed After: All benchmarks complete in ~8 seconds with consistent performance
|
@cyningsun thank you for opening this pr, looking forward to continuing the discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyningsun Thank you for this contribution. I do think we can include it in the library, I do not see any issues with it at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cyningsun left a question regarding the pool balance, the rest looks good.
| if w.tryDeliver(cn, nil) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if putIdleConn returns here, p.freeTurn() won't be called
| _ = cn.Close() | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as well, p.freeTurn() won't be called. We can argue it doesn't matter in this case anyway.
Description
This PR continues the work from the previously closed PR (based on #3418), which was automatically closed after its base branch was merged.
It implements a context pattern for connection creation to prevent premature cancellation caused by short-lived request timeouts, which previously led to connection pool instability under high concurrency.
Solution
Changes
References