Remove prefill logic from resource pool#11002
Conversation
…dded pool benchmark Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
|
I went through RFC as well but not able to find if this change is part of proposal #1, 2 or 3 .. Can you give me more detail why removing pre-fill logic |
It is a small refactor of the pool, and not directly related to RFC. The prefill logic was not used in vitess deployment as the default value is |
| // it will wait till the next resource becomes available or a timeout. | ||
| // A timeout of 0 is an indefinite wait. | ||
| func (rp *ResourcePool) Get(ctx context.Context) (resource Resource, err error) { | ||
| span, ctx := trace.NewSpan(ctx, "ResourcePool.Get") |
There was a problem hiding this comment.
this seems unrelated. Why remove the tracing here?
There was a problem hiding this comment.
The trace is already happening at one higher up level in connpool.Pool
Seems like an unnecessary trace.
span.Annotate("capacity", p.Capacity())
span.Annotate("in_use", p.InUse())
span.Annotate("available", p.Available())
span.Annotate("active", p.Active())
if cp.timeout != 0 {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, cp.timeout)
defer cancel()
}
r, err := p.Get(ctx, nil)Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Description
This PR modifies the resource pool by removing prefill logic and modifying benchmark tests.
Related Issue(s)
Checklist
Deployment Notes