Add LIFO option for getting pool resources #162
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
==========================================
- Coverage 74.28% 71.37% -2.91%
==========================================
Files 6 6
Lines 556 587 +31
==========================================
+ Hits 413 419 +6
- Misses 143 168 +25
☔ View full report in Codecov by Sentry. |
bb8/src/api.rs
Outdated
| #[derive(Debug)] | ||
| pub enum QueueStrategy { | ||
| /// Last in first out | ||
| LIFO, |
There was a problem hiding this comment.
We should spell the variant name Lifo instead of LIFO per the naming guidelines. IMO the comment in its current state isn't that useful, I think it would be helpful to elaborate on the intended effect.
Also, please move the type definition below the Builder impl -- the prevailing style is sort of top-down.
I think it would be useful to derive Default on QueueStrategy, and then initialize Builder::queue_strategy to QueueStrategy::default().
There was a problem hiding this comment.
Thanks for the suggestions, I've implemented them in my latest changes but still failing the build tests. Can't reproduce on my machine so let me know if you have any ideas on how to pass them
There was a problem hiding this comment.
That's right -- I said to derive Default but that's not supported yet by the MSRV. Can you just add a custom Default impl that does the same thing instead? I like to avoid bumping the MSRV where possible.
zainkabani
changed the title
|
@djc Does this seem ready for merge on your end? |
|
Yup, thanks! |
The current strategy that bb8 uses for pulling from the connection queue is FIFO. This means that we are constantly resetting idle timers for connections. This PR adds an option to make the queue a stack so that the pool can reuse the most recently used resources and allow older ones to idle timeout.